linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 1/2] bluetooth: Handle MSFT Monitor Device Event
@ 2021-12-03  7:16 Manish Mandlik
  2021-12-03  7:16 ` [PATCH v7 2/2] bluetooth: Add MGMT Adv Monitor Device Found/Lost events Manish Mandlik
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Manish Mandlik @ 2021-12-03  7:16 UTC (permalink / raw)
  To: marcel, luiz.dentz
  Cc: linux-bluetooth, chromeos-bluetooth-upstreaming, Manish Mandlik,
	Miao-chen Chou, David S. Miller, Jakub Kicinski, Johan Hedberg,
	linux-kernel, netdev

Whenever the controller starts/stops monitoring a bt device, it sends
MSFT Monitor Device event. Add handler to read this vendor event.

Test performed:
- Verified by logs that the MSFT Monitor Device event is received from
  the controller whenever it starts/stops monitoring a device.

Signed-off-by: Manish Mandlik <mmandlik@google.com>
Reviewed-by: Miao-chen Chou <mcchou@google.com>

---

Hello Bt-Maintainers,

As mentioned in the bluez patch series [1], we need to capture the 'MSFT
Monitor Device' event from the controller and pass on the necessary
information 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 series adds support to read the MSFT vendor event
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=583423

Thanks,
Manish.

(no changes since v6)

Changes in v6:
- Fix compiler warning bt_dev_err() missing argument.

Changes in v5:
- Split v4 into two patches.
- Buffer controller Device Found event and maintain the device tracking
  state in the kernel.

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 |  11 +++
 net/bluetooth/hci_core.c         |   1 +
 net/bluetooth/msft.c             | 127 ++++++++++++++++++++++++++++++-
 3 files changed, 138 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 7bae8376fd6f..5ccd19dec77c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -258,6 +258,15 @@ struct adv_info {
 
 #define HCI_ADV_TX_POWER_NO_PREFERENCE 0x7F
 
+struct monitored_device {
+	struct list_head list;
+
+	bdaddr_t bdaddr;
+	__u8     addr_type;
+	__u16    handle;
+	bool     notified;
+};
+
 struct adv_pattern {
 	struct list_head list;
 	__u8 ad_type;
@@ -589,6 +598,8 @@ struct hci_dev {
 
 	struct delayed_work	interleave_scan;
 
+	struct list_head	monitored_devices;
+
 #if IS_ENABLED(CONFIG_BT_LEDS)
 	struct led_trigger	*power_led;
 #endif
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index fdc0dcf8ee36..d4bcd511530a 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2503,6 +2503,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
 	INIT_LIST_HEAD(&hdev->conn_hash.list);
 	INIT_LIST_HEAD(&hdev->adv_instances);
 	INIT_LIST_HEAD(&hdev->blocked_keys);
+	INIT_LIST_HEAD(&hdev->monitored_devices);
 
 	INIT_LIST_HEAD(&hdev->local_codecs);
 	INIT_WORK(&hdev->rx_work, hci_rx_work);
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index 1122097e1e49..aadabe78baf6 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;
@@ -266,6 +274,7 @@ static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
 	struct msft_data *msft = hdev->msft_data;
 	int err;
 	bool pending;
+	struct monitored_device *dev, *tmp;
 
 	if (status)
 		goto done;
@@ -296,6 +305,15 @@ static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
 
 		list_del(&handle_data->list);
 		kfree(handle_data);
+
+		/* Clear any monitored devices by this Adv Monitor */
+		list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
+					 list) {
+			if (dev->handle == handle_data->mgmt_handle) {
+				list_del(&dev->list);
+				kfree(dev);
+			}
+		}
 	}
 
 	/* If remove all monitors is required, we need to continue the process
@@ -538,6 +556,7 @@ void msft_do_close(struct hci_dev *hdev)
 	struct msft_data *msft = hdev->msft_data;
 	struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
 	struct adv_monitor *monitor;
+	struct monitored_device *dev, *tmp_dev;
 
 	if (!msft)
 		return;
@@ -557,6 +576,16 @@ void msft_do_close(struct hci_dev *hdev)
 		list_del(&handle_data->list);
 		kfree(handle_data);
 	}
+
+	hci_dev_lock(hdev);
+
+	/* Clear any devices that are being monitored */
+	list_for_each_entry_safe(dev, tmp_dev, &hdev->monitored_devices, list) {
+		list_del(&dev->list);
+		kfree(dev);
+	}
+
+	hci_dev_unlock(hdev);
 }
 
 void msft_register(struct hci_dev *hdev)
@@ -590,6 +619,90 @@ void msft_unregister(struct hci_dev *hdev)
 	kfree(msft);
 }
 
+/* This function requires the caller holds hdev->lock */
+static void msft_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr,
+			      __u8 addr_type, __u16 mgmt_handle)
+{
+	struct monitored_device *dev;
+
+	dev = kmalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev) {
+		bt_dev_err(hdev, "MSFT vendor event %u: no memory",
+			   MSFT_EV_LE_MONITOR_DEVICE);
+		return;
+	}
+
+	bacpy(&dev->bdaddr, bdaddr);
+	dev->addr_type = addr_type;
+	dev->handle = mgmt_handle;
+	dev->notified = false;
+
+	INIT_LIST_HEAD(&dev->list);
+	list_add(&dev->list, &hdev->monitored_devices);
+}
+
+/* This function requires the caller holds hdev->lock */
+static void msft_device_lost(struct hci_dev *hdev, bdaddr_t *bdaddr,
+			     __u8 addr_type, __u16 mgmt_handle)
+{
+	struct monitored_device *dev, *tmp;
+
+	list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, list) {
+		if (dev->handle == mgmt_handle) {
+			list_del(&dev->list);
+			kfree(dev);
+
+			break;
+		}
+	}
+}
+
+/* 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",
+			   MSFT_EV_LE_MONITOR_DEVICE, ev->addr_type);
+		return;
+	}
+
+	if (ev->monitor_state)
+		msft_device_found(hdev, &ev->bdaddr, addr_type,
+				  handle_data->mgmt_handle);
+	else
+		msft_device_lost(hdev, &ev->bdaddr, addr_type,
+				 handle_data->mgmt_handle);
+}
+
 void msft_vendor_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct msft_data *msft = hdev->msft_data;
@@ -617,10 +730,22 @@ 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;
+
+	default:
+		bt_dev_dbg(hdev, "MSFT vendor event %u", event);
+		break;
+	}
+
+	hci_dev_unlock(hdev);
 }
 
 __u64 msft_get_features(struct hci_dev *hdev)
-- 
2.34.0.384.gca35af8252-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v7 2/2] bluetooth: Add MGMT Adv Monitor Device Found/Lost events
  2021-12-03  7:16 [PATCH v7 1/2] bluetooth: Handle MSFT Monitor Device Event Manish Mandlik
@ 2021-12-03  7:16 ` Manish Mandlik
  2021-12-03 22:18   ` Luiz Augusto von Dentz
  2021-12-03 22:13 ` [PATCH v7 1/2] bluetooth: Handle MSFT Monitor Device Event Luiz Augusto von Dentz
  2021-12-06 13:57 ` Dan Carpenter
  2 siblings, 1 reply; 6+ messages in thread
From: Manish Mandlik @ 2021-12-03  7:16 UTC (permalink / raw)
  To: marcel, luiz.dentz
  Cc: linux-bluetooth, chromeos-bluetooth-upstreaming, Manish Mandlik,
	Miao-chen Chou, David S. Miller, Jakub Kicinski, Johan Hedberg,
	linux-kernel, netdev

This patch introduces two new MGMT events for notifying the bluetoothd
whenever the controller starts/stops monitoring a 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 <mmandlik@google.com>
Reviewed-by: Miao-chen Chou <mcchou@google.com>

---

Changes in v7:
- Refactor mgmt_device_found() to fix stack frame size limit

Changes in v6:
- Fix compiler warning for mgmt_adv_monitor_device_found().

Changes in v5:
- New patch in the series. Split previous patch into two.
- Update the Device Found logic to send existing Device Found event or
  Adv Monitor Device Found event depending on the active scanning state.

 include/net/bluetooth/hci_core.h |   3 +
 include/net/bluetooth/mgmt.h     |  16 +++++
 net/bluetooth/mgmt.c             | 106 +++++++++++++++++++++++++++++--
 net/bluetooth/msft.c             |  15 ++++-
 4 files changed, 134 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 5ccd19dec77c..3b53214ff49f 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -599,6 +599,7 @@ struct hci_dev {
 	struct delayed_work	interleave_scan;
 
 	struct list_head	monitored_devices;
+	bool			advmon_pend_notify;
 
 #if IS_ENABLED(CONFIG_BT_LEDS)
 	struct led_trigger	*power_led;
@@ -1847,6 +1848,8 @@ 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_lost(struct hci_dev *hdev, u16 handle,
+				  bdaddr_t *bdaddr, 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 107b25deae68..99266f7aebdc 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -1104,3 +1104,19 @@ 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;
+	__s8   rssi;
+	__le32 flags;
+	__le16 eir_len;
+	__u8   eir[0];
+} __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 bf989ae03f9f..06e0769f350d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -174,6 +174,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[] = {
@@ -9524,6 +9526,100 @@ static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
 	return true;
 }
 
+void mgmt_adv_monitor_device_lost(struct hci_dev *hdev, u16 handle,
+				  bdaddr_t *bdaddr, u8 addr_type)
+{
+	struct mgmt_ev_adv_monitor_device_lost ev;
+
+	ev.monitor_handle = cpu_to_le16(handle);
+	bacpy(&ev.addr.bdaddr, bdaddr);
+	ev.addr.type = addr_type;
+
+	mgmt_event(MGMT_EV_ADV_MONITOR_DEVICE_LOST, hdev, &ev, sizeof(ev),
+		   NULL);
+}
+
+static void mgmt_adv_monitor_device_found(struct hci_dev *hdev,
+					  struct mgmt_ev_device_found *ev,
+					  size_t ev_size, bool discovering)
+{
+	char buf[518];
+	struct mgmt_ev_adv_monitor_device_found *advmon_ev = (void *)buf;
+	size_t advmon_ev_size;
+	struct monitored_device *dev, *tmp;
+	bool matched = false;
+	bool notified = false;
+
+	/* We have received the Advertisement Report because:
+	 * 1. the kernel has initiated active discovery
+	 * 2. if not, we have pend_le_reports > 0 in which case we are doing
+	 *    passive scanning
+	 * 3. if none of the above is true, we have one or more active
+	 *    Advertisement Monitor
+	 *
+	 * For case 1 and 2, report all advertisements via MGMT_EV_DEVICE_FOUND
+	 * and report ONLY one advertisement per device for the matched Monitor
+	 * via MGMT_EV_ADV_MONITOR_DEVICE_FOUND event.
+	 *
+	 * For case 3, since we are not active scanning and all advertisements
+	 * received are due to a matched Advertisement Monitor, report all
+	 * advertisements ONLY via MGMT_EV_ADV_MONITOR_DEVICE_FOUND event.
+	 */
+	if (discovering) {
+		mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
+
+		if (!hdev->advmon_pend_notify)
+			return;
+	}
+
+	/* Make sure that the buffer is big enough */
+	advmon_ev_size = ev_size + (sizeof(*advmon_ev) - sizeof(*ev));
+	if (advmon_ev_size > sizeof(buf))
+		return;
+
+	/* ADV_MONITOR_DEVICE_FOUND is similar to DEVICE_FOUND event except
+	 * that it also has 'monitor_handle'. Make a copy of DEVICE_FOUND and
+	 * store monitor_handle of the matched monitor.
+	 */
+	memcpy(&advmon_ev->addr, ev, ev_size);
+
+	hdev->advmon_pend_notify = false;
+
+	list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, list) {
+		if (!bacmp(&dev->bdaddr, &advmon_ev->addr.bdaddr)) {
+			matched = true;
+
+			if (!dev->notified) {
+				advmon_ev->monitor_handle =
+						cpu_to_le16(dev->handle);
+
+				mgmt_event(MGMT_EV_ADV_MONITOR_DEVICE_FOUND,
+					   hdev, advmon_ev, advmon_ev_size,
+					   NULL);
+
+				notified = true;
+				dev->notified = true;
+			}
+		}
+
+		if (!dev->notified)
+			hdev->advmon_pend_notify = true;
+	}
+
+	if (!discovering &&
+	    ((matched && !notified) || !msft_monitor_supported(hdev))) {
+		/* Handle 0 indicates that we are not active scanning and this
+		 * is a subsequent advertisement report for an already matched
+		 * Advertisement Monitor or the controller offloading support
+		 * is not available.
+		 */
+		advmon_ev->monitor_handle = 0;
+
+		mgmt_event(MGMT_EV_ADV_MONITOR_DEVICE_FOUND, hdev, advmon_ev,
+			   advmon_ev_size, NULL);
+	}
+}
+
 void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
 		       u8 addr_type, u8 *dev_class, s8 rssi, u32 flags,
 		       u8 *eir, u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
@@ -9531,6 +9627,7 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
 	char buf[512];
 	struct mgmt_ev_device_found *ev = (void *)buf;
 	size_t ev_size;
+	bool report_device_found = hci_discovery_active(hdev);
 
 	/* Don't send events for a non-kernel initiated discovery. With
 	 * LE one exception is if we have pend_le_reports > 0 in which
@@ -9539,11 +9636,10 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
 	if (!hci_discovery_active(hdev)) {
 		if (link_type == ACL_LINK)
 			return;
-		if (link_type == LE_LINK &&
-		    list_empty(&hdev->pend_le_reports) &&
-		    !hci_is_adv_monitoring(hdev)) {
+		if (link_type == LE_LINK && !list_empty(&hdev->pend_le_reports))
+			report_device_found = true;
+		else if (!hci_is_adv_monitoring(hdev))
 			return;
-		}
 	}
 
 	if (hdev->discovery.result_filtering) {
@@ -9606,7 +9702,7 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
 	ev->eir_len = cpu_to_le16(eir_len + scan_rsp_len);
 	ev_size = sizeof(*ev) + eir_len + scan_rsp_len;
 
-	mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
+	mgmt_adv_monitor_device_found(hdev, ev, ev_size, report_device_found);
 }
 
 void mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index aadabe78baf6..3e2385562d2b 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -579,8 +579,16 @@ void msft_do_close(struct hci_dev *hdev)
 
 	hci_dev_lock(hdev);
 
-	/* Clear any devices that are being monitored */
+	/* Clear any devices that are being monitored and notify device lost */
+
+	hdev->advmon_pend_notify = false;
+
 	list_for_each_entry_safe(dev, tmp_dev, &hdev->monitored_devices, list) {
+		if (dev->notified)
+			mgmt_adv_monitor_device_lost(hdev, dev->handle,
+						     &dev->bdaddr,
+						     dev->addr_type);
+
 		list_del(&dev->list);
 		kfree(dev);
 	}
@@ -639,6 +647,7 @@ static void msft_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr,
 
 	INIT_LIST_HEAD(&dev->list);
 	list_add(&dev->list, &hdev->monitored_devices);
+	hdev->advmon_pend_notify = true;
 }
 
 /* This function requires the caller holds hdev->lock */
@@ -649,6 +658,10 @@ static void msft_device_lost(struct hci_dev *hdev, bdaddr_t *bdaddr,
 
 	list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, list) {
 		if (dev->handle == mgmt_handle) {
+			if (dev->notified)
+				mgmt_adv_monitor_device_lost(hdev, mgmt_handle,
+							     bdaddr, addr_type);
+
 			list_del(&dev->list);
 			kfree(dev);
 
-- 
2.34.0.384.gca35af8252-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v7 1/2] bluetooth: Handle MSFT Monitor Device Event
  2021-12-03  7:16 [PATCH v7 1/2] bluetooth: Handle MSFT Monitor Device Event Manish Mandlik
  2021-12-03  7:16 ` [PATCH v7 2/2] bluetooth: Add MGMT Adv Monitor Device Found/Lost events Manish Mandlik
@ 2021-12-03 22:13 ` Luiz Augusto von Dentz
  2021-12-06 13:57 ` Dan Carpenter
  2 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2021-12-03 22:13 UTC (permalink / raw)
  To: Manish Mandlik
  Cc: Marcel Holtmann, linux-bluetooth, ChromeOS Bluetooth Upstreaming,
	Miao-chen Chou, David S. Miller, Jakub Kicinski, Johan Hedberg,
	Linux Kernel Mailing List, open list:NETWORKING [GENERAL]

Hi Manish,

On Thu, Dec 2, 2021 at 11:16 PM Manish Mandlik <mmandlik@google.com> wrote:
>
> Whenever the controller starts/stops monitoring a bt device, it sends
> MSFT Monitor Device event. Add handler to read this vendor event.
>
> Test performed:
> - Verified by logs that the MSFT Monitor Device event is received from
>   the controller whenever it starts/stops monitoring a device.
>
> Signed-off-by: Manish Mandlik <mmandlik@google.com>
> Reviewed-by: Miao-chen Chou <mcchou@google.com>
>
> ---
>
> Hello Bt-Maintainers,
>
> As mentioned in the bluez patch series [1], we need to capture the 'MSFT
> Monitor Device' event from the controller and pass on the necessary
> information 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 series adds support to read the MSFT vendor event
> 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=583423
>
> Thanks,
> Manish.
>
> (no changes since v6)
>
> Changes in v6:
> - Fix compiler warning bt_dev_err() missing argument.
>
> Changes in v5:
> - Split v4 into two patches.
> - Buffer controller Device Found event and maintain the device tracking
>   state in the kernel.
>
> 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 |  11 +++
>  net/bluetooth/hci_core.c         |   1 +
>  net/bluetooth/msft.c             | 127 ++++++++++++++++++++++++++++++-
>  3 files changed, 138 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 7bae8376fd6f..5ccd19dec77c 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -258,6 +258,15 @@ struct adv_info {
>
>  #define HCI_ADV_TX_POWER_NO_PREFERENCE 0x7F
>
> +struct monitored_device {
> +       struct list_head list;
> +
> +       bdaddr_t bdaddr;
> +       __u8     addr_type;
> +       __u16    handle;
> +       bool     notified;
> +};
> +
>  struct adv_pattern {
>         struct list_head list;
>         __u8 ad_type;
> @@ -589,6 +598,8 @@ struct hci_dev {
>
>         struct delayed_work     interleave_scan;
>
> +       struct list_head        monitored_devices;
> +
>  #if IS_ENABLED(CONFIG_BT_LEDS)
>         struct led_trigger      *power_led;
>  #endif
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index fdc0dcf8ee36..d4bcd511530a 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2503,6 +2503,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
>         INIT_LIST_HEAD(&hdev->conn_hash.list);
>         INIT_LIST_HEAD(&hdev->adv_instances);
>         INIT_LIST_HEAD(&hdev->blocked_keys);
> +       INIT_LIST_HEAD(&hdev->monitored_devices);
>
>         INIT_LIST_HEAD(&hdev->local_codecs);
>         INIT_WORK(&hdev->rx_work, hci_rx_work);
> diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> index 1122097e1e49..aadabe78baf6 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;
> @@ -266,6 +274,7 @@ static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
>         struct msft_data *msft = hdev->msft_data;
>         int err;
>         bool pending;
> +       struct monitored_device *dev, *tmp;
>
>         if (status)
>                 goto done;
> @@ -296,6 +305,15 @@ static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
>
>                 list_del(&handle_data->list);
>                 kfree(handle_data);
> +
> +               /* Clear any monitored devices by this Adv Monitor */
> +               list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
> +                                        list) {
> +                       if (dev->handle == handle_data->mgmt_handle) {
> +                               list_del(&dev->list);
> +                               kfree(dev);
> +                       }
> +               }
>         }
>
>         /* If remove all monitors is required, we need to continue the process
> @@ -538,6 +556,7 @@ void msft_do_close(struct hci_dev *hdev)
>         struct msft_data *msft = hdev->msft_data;
>         struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
>         struct adv_monitor *monitor;
> +       struct monitored_device *dev, *tmp_dev;
>
>         if (!msft)
>                 return;
> @@ -557,6 +576,16 @@ void msft_do_close(struct hci_dev *hdev)
>                 list_del(&handle_data->list);
>                 kfree(handle_data);
>         }
> +
> +       hci_dev_lock(hdev);
> +
> +       /* Clear any devices that are being monitored */
> +       list_for_each_entry_safe(dev, tmp_dev, &hdev->monitored_devices, list) {
> +               list_del(&dev->list);
> +               kfree(dev);
> +       }
> +
> +       hci_dev_unlock(hdev);
>  }
>
>  void msft_register(struct hci_dev *hdev)
> @@ -590,6 +619,90 @@ void msft_unregister(struct hci_dev *hdev)
>         kfree(msft);
>  }
>
> +/* This function requires the caller holds hdev->lock */
> +static void msft_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr,
> +                             __u8 addr_type, __u16 mgmt_handle)
> +{
> +       struct monitored_device *dev;
> +
> +       dev = kmalloc(sizeof(*dev), GFP_KERNEL);
> +       if (!dev) {
> +               bt_dev_err(hdev, "MSFT vendor event %u: no memory",
> +                          MSFT_EV_LE_MONITOR_DEVICE);
> +               return;
> +       }
> +
> +       bacpy(&dev->bdaddr, bdaddr);
> +       dev->addr_type = addr_type;
> +       dev->handle = mgmt_handle;
> +       dev->notified = false;
> +
> +       INIT_LIST_HEAD(&dev->list);
> +       list_add(&dev->list, &hdev->monitored_devices);
> +}
> +
> +/* This function requires the caller holds hdev->lock */
> +static void msft_device_lost(struct hci_dev *hdev, bdaddr_t *bdaddr,
> +                            __u8 addr_type, __u16 mgmt_handle)
> +{
> +       struct monitored_device *dev, *tmp;
> +
> +       list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, list) {
> +               if (dev->handle == mgmt_handle) {
> +                       list_del(&dev->list);
> +                       kfree(dev);
> +
> +                       break;
> +               }
> +       }
> +}
> +
> +/* 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));

Please use skb_pull_data now that it is available in bluetooth-next.

> +       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",
> +                          MSFT_EV_LE_MONITOR_DEVICE, ev->addr_type);
> +               return;
> +       }
> +
> +       if (ev->monitor_state)
> +               msft_device_found(hdev, &ev->bdaddr, addr_type,
> +                                 handle_data->mgmt_handle);
> +       else
> +               msft_device_lost(hdev, &ev->bdaddr, addr_type,
> +                                handle_data->mgmt_handle);
> +}
> +
>  void msft_vendor_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>         struct msft_data *msft = hdev->msft_data;
> @@ -617,10 +730,22 @@ 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;
> +
> +       default:
> +               bt_dev_dbg(hdev, "MSFT vendor event %u", event);
> +               break;
> +       }
> +
> +       hci_dev_unlock(hdev);
>  }
>
>  __u64 msft_get_features(struct hci_dev *hdev)
> --
> 2.34.0.384.gca35af8252-goog
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v7 2/2] bluetooth: Add MGMT Adv Monitor Device Found/Lost events
  2021-12-03  7:16 ` [PATCH v7 2/2] bluetooth: Add MGMT Adv Monitor Device Found/Lost events Manish Mandlik
@ 2021-12-03 22:18   ` Luiz Augusto von Dentz
       [not found]     ` <CAGPPCLDreZm2m_ZmymVumu5s1tDeXi7-UGFDFnxd8ZuzW6oVPA@mail.gmail.com>
  0 siblings, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2021-12-03 22:18 UTC (permalink / raw)
  To: Manish Mandlik
  Cc: Marcel Holtmann, linux-bluetooth, ChromeOS Bluetooth Upstreaming,
	Miao-chen Chou, David S. Miller, Jakub Kicinski, Johan Hedberg,
	Linux Kernel Mailing List, open list:NETWORKING [GENERAL]

Hi Manish,

On Thu, Dec 2, 2021 at 11:16 PM Manish Mandlik <mmandlik@google.com> wrote:
>
> This patch introduces two new MGMT events for notifying the bluetoothd
> whenever the controller starts/stops monitoring a 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 <mmandlik@google.com>
> Reviewed-by: Miao-chen Chou <mcchou@google.com>
>
> ---
>
> Changes in v7:
> - Refactor mgmt_device_found() to fix stack frame size limit
>
> Changes in v6:
> - Fix compiler warning for mgmt_adv_monitor_device_found().
>
> Changes in v5:
> - New patch in the series. Split previous patch into two.
> - Update the Device Found logic to send existing Device Found event or
>   Adv Monitor Device Found event depending on the active scanning state.
>
>  include/net/bluetooth/hci_core.h |   3 +
>  include/net/bluetooth/mgmt.h     |  16 +++++
>  net/bluetooth/mgmt.c             | 106 +++++++++++++++++++++++++++++--
>  net/bluetooth/msft.c             |  15 ++++-
>  4 files changed, 134 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 5ccd19dec77c..3b53214ff49f 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -599,6 +599,7 @@ struct hci_dev {
>         struct delayed_work     interleave_scan;
>
>         struct list_head        monitored_devices;
> +       bool                    advmon_pend_notify;
>
>  #if IS_ENABLED(CONFIG_BT_LEDS)
>         struct led_trigger      *power_led;
> @@ -1847,6 +1848,8 @@ 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_lost(struct hci_dev *hdev, u16 handle,
> +                                 bdaddr_t *bdaddr, 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 107b25deae68..99266f7aebdc 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -1104,3 +1104,19 @@ 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;
> +       __s8   rssi;
> +       __le32 flags;
> +       __le16 eir_len;
> +       __u8   eir[0];
> +} __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 bf989ae03f9f..06e0769f350d 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -174,6 +174,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[] = {
> @@ -9524,6 +9526,100 @@ static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
>         return true;
>  }
>
> +void mgmt_adv_monitor_device_lost(struct hci_dev *hdev, u16 handle,
> +                                 bdaddr_t *bdaddr, u8 addr_type)
> +{
> +       struct mgmt_ev_adv_monitor_device_lost ev;
> +
> +       ev.monitor_handle = cpu_to_le16(handle);
> +       bacpy(&ev.addr.bdaddr, bdaddr);
> +       ev.addr.type = addr_type;
> +
> +       mgmt_event(MGMT_EV_ADV_MONITOR_DEVICE_LOST, hdev, &ev, sizeof(ev),
> +                  NULL);
> +}
> +
> +static void mgmt_adv_monitor_device_found(struct hci_dev *hdev,
> +                                         struct mgmt_ev_device_found *ev,
> +                                         size_t ev_size, bool discovering)
> +{
> +       char buf[518];

We should try to avoid declaring such big stack variable, instead Im
working on a set the should enable you to use skb_put/skb_put_data
directly into the skb, here are the current changes I have for
device_found:

https://gist.github.com/Vudentz/ffce584912eb7dc4add9c3bb25466fa5

I'm addressing Marcel's comments and will send the set to the list later today.

> +       struct mgmt_ev_adv_monitor_device_found *advmon_ev = (void *)buf;
> +       size_t advmon_ev_size;
> +       struct monitored_device *dev, *tmp;
> +       bool matched = false;
> +       bool notified = false;
> +
> +       /* We have received the Advertisement Report because:
> +        * 1. the kernel has initiated active discovery
> +        * 2. if not, we have pend_le_reports > 0 in which case we are doing
> +        *    passive scanning
> +        * 3. if none of the above is true, we have one or more active
> +        *    Advertisement Monitor
> +        *
> +        * For case 1 and 2, report all advertisements via MGMT_EV_DEVICE_FOUND
> +        * and report ONLY one advertisement per device for the matched Monitor
> +        * via MGMT_EV_ADV_MONITOR_DEVICE_FOUND event.
> +        *
> +        * For case 3, since we are not active scanning and all advertisements
> +        * received are due to a matched Advertisement Monitor, report all
> +        * advertisements ONLY via MGMT_EV_ADV_MONITOR_DEVICE_FOUND event.
> +        */
> +       if (discovering) {
> +               mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
> +
> +               if (!hdev->advmon_pend_notify)
> +                       return;
> +       }
> +
> +       /* Make sure that the buffer is big enough */
> +       advmon_ev_size = ev_size + (sizeof(*advmon_ev) - sizeof(*ev));
> +       if (advmon_ev_size > sizeof(buf))
> +               return;
> +
> +       /* ADV_MONITOR_DEVICE_FOUND is similar to DEVICE_FOUND event except
> +        * that it also has 'monitor_handle'. Make a copy of DEVICE_FOUND and
> +        * store monitor_handle of the matched monitor.
> +        */
> +       memcpy(&advmon_ev->addr, ev, ev_size);
> +
> +       hdev->advmon_pend_notify = false;
> +
> +       list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, list) {
> +               if (!bacmp(&dev->bdaddr, &advmon_ev->addr.bdaddr)) {
> +                       matched = true;
> +
> +                       if (!dev->notified) {
> +                               advmon_ev->monitor_handle =
> +                                               cpu_to_le16(dev->handle);
> +
> +                               mgmt_event(MGMT_EV_ADV_MONITOR_DEVICE_FOUND,
> +                                          hdev, advmon_ev, advmon_ev_size,
> +                                          NULL);
> +
> +                               notified = true;
> +                               dev->notified = true;
> +                       }
> +               }
> +
> +               if (!dev->notified)
> +                       hdev->advmon_pend_notify = true;
> +       }
> +
> +       if (!discovering &&
> +           ((matched && !notified) || !msft_monitor_supported(hdev))) {
> +               /* Handle 0 indicates that we are not active scanning and this
> +                * is a subsequent advertisement report for an already matched
> +                * Advertisement Monitor or the controller offloading support
> +                * is not available.
> +                */
> +               advmon_ev->monitor_handle = 0;
> +
> +               mgmt_event(MGMT_EV_ADV_MONITOR_DEVICE_FOUND, hdev, advmon_ev,
> +                          advmon_ev_size, NULL);
> +       }
> +}
> +
>  void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>                        u8 addr_type, u8 *dev_class, s8 rssi, u32 flags,
>                        u8 *eir, u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
> @@ -9531,6 +9627,7 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>         char buf[512];
>         struct mgmt_ev_device_found *ev = (void *)buf;
>         size_t ev_size;
> +       bool report_device_found = hci_discovery_active(hdev);
>
>         /* Don't send events for a non-kernel initiated discovery. With
>          * LE one exception is if we have pend_le_reports > 0 in which
> @@ -9539,11 +9636,10 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>         if (!hci_discovery_active(hdev)) {
>                 if (link_type == ACL_LINK)
>                         return;
> -               if (link_type == LE_LINK &&
> -                   list_empty(&hdev->pend_le_reports) &&
> -                   !hci_is_adv_monitoring(hdev)) {
> +               if (link_type == LE_LINK && !list_empty(&hdev->pend_le_reports))
> +                       report_device_found = true;
> +               else if (!hci_is_adv_monitoring(hdev))
>                         return;
> -               }
>         }
>
>         if (hdev->discovery.result_filtering) {
> @@ -9606,7 +9702,7 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>         ev->eir_len = cpu_to_le16(eir_len + scan_rsp_len);
>         ev_size = sizeof(*ev) + eir_len + scan_rsp_len;
>
> -       mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
> +       mgmt_adv_monitor_device_found(hdev, ev, ev_size, report_device_found);
>  }
>
>  void mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
> diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> index aadabe78baf6..3e2385562d2b 100644
> --- a/net/bluetooth/msft.c
> +++ b/net/bluetooth/msft.c
> @@ -579,8 +579,16 @@ void msft_do_close(struct hci_dev *hdev)
>
>         hci_dev_lock(hdev);
>
> -       /* Clear any devices that are being monitored */
> +       /* Clear any devices that are being monitored and notify device lost */
> +
> +       hdev->advmon_pend_notify = false;
> +
>         list_for_each_entry_safe(dev, tmp_dev, &hdev->monitored_devices, list) {
> +               if (dev->notified)
> +                       mgmt_adv_monitor_device_lost(hdev, dev->handle,
> +                                                    &dev->bdaddr,
> +                                                    dev->addr_type);
> +
>                 list_del(&dev->list);
>                 kfree(dev);
>         }
> @@ -639,6 +647,7 @@ static void msft_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr,
>
>         INIT_LIST_HEAD(&dev->list);
>         list_add(&dev->list, &hdev->monitored_devices);
> +       hdev->advmon_pend_notify = true;
>  }
>
>  /* This function requires the caller holds hdev->lock */
> @@ -649,6 +658,10 @@ static void msft_device_lost(struct hci_dev *hdev, bdaddr_t *bdaddr,
>
>         list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, list) {
>                 if (dev->handle == mgmt_handle) {
> +                       if (dev->notified)
> +                               mgmt_adv_monitor_device_lost(hdev, mgmt_handle,
> +                                                            bdaddr, addr_type);
> +
>                         list_del(&dev->list);
>                         kfree(dev);
>
> --
> 2.34.0.384.gca35af8252-goog
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v7 1/2] bluetooth: Handle MSFT Monitor Device Event
  2021-12-03  7:16 [PATCH v7 1/2] bluetooth: Handle MSFT Monitor Device Event Manish Mandlik
  2021-12-03  7:16 ` [PATCH v7 2/2] bluetooth: Add MGMT Adv Monitor Device Found/Lost events Manish Mandlik
  2021-12-03 22:13 ` [PATCH v7 1/2] bluetooth: Handle MSFT Monitor Device Event Luiz Augusto von Dentz
@ 2021-12-06 13:57 ` Dan Carpenter
  2 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2021-12-06 13:57 UTC (permalink / raw)
  To: kbuild, Manish Mandlik, marcel, luiz.dentz
  Cc: lkp, kbuild-all, linux-bluetooth, chromeos-bluetooth-upstreaming,
	Manish Mandlik, Miao-chen Chou, Jakub Kicinski, Johan Hedberg,
	linux-kernel, netdev

Hi Manish,

url:    https://github.com/0day-ci/linux/commits/Manish-Mandlik/bluetooth-Handle-MSFT-Monitor-Device-Event/20211203-151659
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: x86_64-randconfig-m001-20211203 (https://download.01.org/0day-ci/archive/20211205/202112050416.RYsEcWkk-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
net/bluetooth/msft.c:312 msft_le_cancel_monitor_advertisement_cb() error: dereferencing freed memory 'handle_data'

vim +/handle_data +312 net/bluetooth/msft.c

182ee45da083db Luiz Augusto von Dentz 2021-10-27  266  static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
182ee45da083db Luiz Augusto von Dentz 2021-10-27  267  						    u8 status, u16 opcode,
182ee45da083db Luiz Augusto von Dentz 2021-10-27  268  						    struct sk_buff *skb)
ce81843be24e9d Manish Mandlik         2021-09-21  269  {
182ee45da083db Luiz Augusto von Dentz 2021-10-27  270  	struct msft_cp_le_cancel_monitor_advertisement *cp;
182ee45da083db Luiz Augusto von Dentz 2021-10-27  271  	struct msft_rp_le_cancel_monitor_advertisement *rp;
182ee45da083db Luiz Augusto von Dentz 2021-10-27  272  	struct adv_monitor *monitor;
182ee45da083db Luiz Augusto von Dentz 2021-10-27  273  	struct msft_monitor_advertisement_handle_data *handle_data;
ce81843be24e9d Manish Mandlik         2021-09-21  274  	struct msft_data *msft = hdev->msft_data;
182ee45da083db Luiz Augusto von Dentz 2021-10-27  275  	int err;
182ee45da083db Luiz Augusto von Dentz 2021-10-27  276  	bool pending;
eb96f195e598b7 Manish Mandlik         2021-12-02  277  	struct monitored_device *dev, *tmp;
ce81843be24e9d Manish Mandlik         2021-09-21  278  
182ee45da083db Luiz Augusto von Dentz 2021-10-27  279  	if (status)
182ee45da083db Luiz Augusto von Dentz 2021-10-27  280  		goto done;
182ee45da083db Luiz Augusto von Dentz 2021-10-27  281  
182ee45da083db Luiz Augusto von Dentz 2021-10-27  282  	rp = (struct msft_rp_le_cancel_monitor_advertisement *)skb->data;
182ee45da083db Luiz Augusto von Dentz 2021-10-27  283  	if (skb->len < sizeof(*rp)) {
182ee45da083db Luiz Augusto von Dentz 2021-10-27  284  		status = HCI_ERROR_UNSPECIFIED;
182ee45da083db Luiz Augusto von Dentz 2021-10-27  285  		goto done;
182ee45da083db Luiz Augusto von Dentz 2021-10-27  286  	}
182ee45da083db Luiz Augusto von Dentz 2021-10-27  287  
182ee45da083db Luiz Augusto von Dentz 2021-10-27  288  	hci_dev_lock(hdev);
182ee45da083db Luiz Augusto von Dentz 2021-10-27  289  
182ee45da083db Luiz Augusto von Dentz 2021-10-27  290  	cp = hci_sent_cmd_data(hdev, hdev->msft_opcode);
182ee45da083db Luiz Augusto von Dentz 2021-10-27  291  	handle_data = msft_find_handle_data(hdev, cp->handle, false);
182ee45da083db Luiz Augusto von Dentz 2021-10-27  292  
182ee45da083db Luiz Augusto von Dentz 2021-10-27  293  	if (handle_data) {
182ee45da083db Luiz Augusto von Dentz 2021-10-27  294  		monitor = idr_find(&hdev->adv_monitors_idr,
182ee45da083db Luiz Augusto von Dentz 2021-10-27  295  				   handle_data->mgmt_handle);
182ee45da083db Luiz Augusto von Dentz 2021-10-27  296  
182ee45da083db Luiz Augusto von Dentz 2021-10-27  297  		if (monitor && monitor->state == ADV_MONITOR_STATE_OFFLOADED)
182ee45da083db Luiz Augusto von Dentz 2021-10-27  298  			monitor->state = ADV_MONITOR_STATE_REGISTERED;
182ee45da083db Luiz Augusto von Dentz 2021-10-27  299  
182ee45da083db Luiz Augusto von Dentz 2021-10-27  300  		/* Do not free the monitor if it is being removed due to
182ee45da083db Luiz Augusto von Dentz 2021-10-27  301  		 * suspend. It will be re-monitored on resume.
182ee45da083db Luiz Augusto von Dentz 2021-10-27  302  		 */
182ee45da083db Luiz Augusto von Dentz 2021-10-27  303  		if (monitor && !msft->suspending)
182ee45da083db Luiz Augusto von Dentz 2021-10-27  304  			hci_free_adv_monitor(hdev, monitor);
182ee45da083db Luiz Augusto von Dentz 2021-10-27  305  
182ee45da083db Luiz Augusto von Dentz 2021-10-27  306  		list_del(&handle_data->list);
182ee45da083db Luiz Augusto von Dentz 2021-10-27  307  		kfree(handle_data);
                                                                ^^^^^^^^^^^^^^^^^^
Free

eb96f195e598b7 Manish Mandlik         2021-12-02  308  
eb96f195e598b7 Manish Mandlik         2021-12-02  309  		/* Clear any monitored devices by this Adv Monitor */
eb96f195e598b7 Manish Mandlik         2021-12-02  310  		list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
eb96f195e598b7 Manish Mandlik         2021-12-02  311  					 list) {
eb96f195e598b7 Manish Mandlik         2021-12-02 @312  			if (dev->handle == handle_data->mgmt_handle) {
                                                                                           ^^^^^^^^^^^^^^^^^^^^^^^^
Use after free.

eb96f195e598b7 Manish Mandlik         2021-12-02  313  				list_del(&dev->list);
eb96f195e598b7 Manish Mandlik         2021-12-02  314  				kfree(dev);
eb96f195e598b7 Manish Mandlik         2021-12-02  315  			}
eb96f195e598b7 Manish Mandlik         2021-12-02  316  		}
182ee45da083db Luiz Augusto von Dentz 2021-10-27  317  	}
182ee45da083db Luiz Augusto von Dentz 2021-10-27  318  
182ee45da083db Luiz Augusto von Dentz 2021-10-27  319  	/* If remove all monitors is required, we need to continue the process
182ee45da083db Luiz Augusto von Dentz 2021-10-27  320  	 * here because the earlier it was paused when waiting for the
182ee45da083db Luiz Augusto von Dentz 2021-10-27  321  	 * response from controller.
182ee45da083db Luiz Augusto von Dentz 2021-10-27  322  	 */
182ee45da083db Luiz Augusto von Dentz 2021-10-27  323  	if (msft->pending_remove_handle == 0) {
182ee45da083db Luiz Augusto von Dentz 2021-10-27  324  		pending = hci_remove_all_adv_monitor(hdev, &err);
182ee45da083db Luiz Augusto von Dentz 2021-10-27  325  		if (pending) {
182ee45da083db Luiz Augusto von Dentz 2021-10-27  326  			hci_dev_unlock(hdev);
ce81843be24e9d Manish Mandlik         2021-09-21  327  			return;
182ee45da083db Luiz Augusto von Dentz 2021-10-27  328  		}
182ee45da083db Luiz Augusto von Dentz 2021-10-27  329  
182ee45da083db Luiz Augusto von Dentz 2021-10-27  330  		if (err)
182ee45da083db Luiz Augusto von Dentz 2021-10-27  331  			status = HCI_ERROR_UNSPECIFIED;
182ee45da083db Luiz Augusto von Dentz 2021-10-27  332  	}
182ee45da083db Luiz Augusto von Dentz 2021-10-27  333  
182ee45da083db Luiz Augusto von Dentz 2021-10-27  334  	hci_dev_unlock(hdev);
182ee45da083db Luiz Augusto von Dentz 2021-10-27  335  
182ee45da083db Luiz Augusto von Dentz 2021-10-27  336  done:
182ee45da083db Luiz Augusto von Dentz 2021-10-27  337  	if (!msft->suspending)
182ee45da083db Luiz Augusto von Dentz 2021-10-27  338  		hci_remove_adv_monitor_complete(hdev, status);
182ee45da083db Luiz Augusto von Dentz 2021-10-27  339  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v7 2/2] bluetooth: Add MGMT Adv Monitor Device Found/Lost events
       [not found]     ` <CAGPPCLDreZm2m_ZmymVumu5s1tDeXi7-UGFDFnxd8ZuzW6oVPA@mail.gmail.com>
@ 2021-12-13 17:20       ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2021-12-13 17:20 UTC (permalink / raw)
  To: Manish Mandlik
  Cc: Marcel Holtmann, linux-bluetooth, ChromeOS Bluetooth Upstreaming,
	Miao-chen Chou, David S. Miller, Jakub Kicinski, Johan Hedberg,
	Linux Kernel Mailing List, open list:NETWORKING [GENERAL]

Hi Manish,

On Mon, Dec 6, 2021 at 8:39 AM Manish Mandlik <mmandlik@google.com> wrote:
>
> Hi Luiz,
>
> On Fri, Dec 3, 2021 at 5:19 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>
>> Hi Manish,
>>
>> On Thu, Dec 2, 2021 at 11:16 PM Manish Mandlik <mmandlik@google.com> wrote:
>> >
>> > This patch introduces two new MGMT events for notifying the bluetoothd
>> > whenever the controller starts/stops monitoring a 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 <mmandlik@google.com>
>> > Reviewed-by: Miao-chen Chou <mcchou@google.com>
>> >
>> > ---
>> >
>> > Changes in v7:
>> > - Refactor mgmt_device_found() to fix stack frame size limit
>> >
>> > Changes in v6:
>> > - Fix compiler warning for mgmt_adv_monitor_device_found().
>> >
>> > Changes in v5:
>> > - New patch in the series. Split previous patch into two.
>> > - Update the Device Found logic to send existing Device Found event or
>> >   Adv Monitor Device Found event depending on the active scanning state.
>> >
>> >  include/net/bluetooth/hci_core.h |   3 +
>> >  include/net/bluetooth/mgmt.h     |  16 +++++
>> >  net/bluetooth/mgmt.c             | 106 +++++++++++++++++++++++++++++--
>> >  net/bluetooth/msft.c             |  15 ++++-
>> >  4 files changed, 134 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> > index 5ccd19dec77c..3b53214ff49f 100644
>> > --- a/include/net/bluetooth/hci_core.h
>> > +++ b/include/net/bluetooth/hci_core.h
>> > @@ -599,6 +599,7 @@ struct hci_dev {
>> >         struct delayed_work     interleave_scan;
>> >
>> >         struct list_head        monitored_devices;
>> > +       bool                    advmon_pend_notify;
>> >
>> >  #if IS_ENABLED(CONFIG_BT_LEDS)
>> >         struct led_trigger      *power_led;
>> > @@ -1847,6 +1848,8 @@ 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_lost(struct hci_dev *hdev, u16 handle,
>> > +                                 bdaddr_t *bdaddr, 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 107b25deae68..99266f7aebdc 100644
>> > --- a/include/net/bluetooth/mgmt.h
>> > +++ b/include/net/bluetooth/mgmt.h
>> > @@ -1104,3 +1104,19 @@ 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;
>> > +       __s8   rssi;
>> > +       __le32 flags;
>> > +       __le16 eir_len;
>> > +       __u8   eir[0];
>> > +} __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 bf989ae03f9f..06e0769f350d 100644
>> > --- a/net/bluetooth/mgmt.c
>> > +++ b/net/bluetooth/mgmt.c
>> > @@ -174,6 +174,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[] = {
>> > @@ -9524,6 +9526,100 @@ static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
>> >         return true;
>> >  }
>> >
>> > +void mgmt_adv_monitor_device_lost(struct hci_dev *hdev, u16 handle,
>> > +                                 bdaddr_t *bdaddr, u8 addr_type)
>> > +{
>> > +       struct mgmt_ev_adv_monitor_device_lost ev;
>> > +
>> > +       ev.monitor_handle = cpu_to_le16(handle);
>> > +       bacpy(&ev.addr.bdaddr, bdaddr);
>> > +       ev.addr.type = addr_type;
>> > +
>> > +       mgmt_event(MGMT_EV_ADV_MONITOR_DEVICE_LOST, hdev, &ev, sizeof(ev),
>> > +                  NULL);
>> > +}
>> > +
>> > +static void mgmt_adv_monitor_device_found(struct hci_dev *hdev,
>> > +                                         struct mgmt_ev_device_found *ev,
>> > +                                         size_t ev_size, bool discovering)
>> > +{
>> > +       char buf[518];
>>
>> We should try to avoid declaring such big stack variable, instead Im
>> working on a set the should enable you to use skb_put/skb_put_data
>> directly into the skb, here are the current changes I have for
>> device_found:
>>
>> https://gist.github.com/Vudentz/ffce584912eb7dc4add9c3bb25466fa5
>>
>> I'm addressing Marcel's comments and will send the set to the list later today.
>
> Please let me know once these changes land bluetooth-next, I'll update my patch and send out the new revision.

Those changes are already in bluetooth-next.

>>
>>
>> > +       struct mgmt_ev_adv_monitor_device_found *advmon_ev = (void *)buf;
>> > +       size_t advmon_ev_size;
>> > +       struct monitored_device *dev, *tmp;
>> > +       bool matched = false;
>> > +       bool notified = false;
>> > +
>> > +       /* We have received the Advertisement Report because:
>> > +        * 1. the kernel has initiated active discovery
>> > +        * 2. if not, we have pend_le_reports > 0 in which case we are doing
>> > +        *    passive scanning
>> > +        * 3. if none of the above is true, we have one or more active
>> > +        *    Advertisement Monitor
>> > +        *
>> > +        * For case 1 and 2, report all advertisements via MGMT_EV_DEVICE_FOUND
>> > +        * and report ONLY one advertisement per device for the matched Monitor
>> > +        * via MGMT_EV_ADV_MONITOR_DEVICE_FOUND event.
>> > +        *
>> > +        * For case 3, since we are not active scanning and all advertisements
>> > +        * received are due to a matched Advertisement Monitor, report all
>> > +        * advertisements ONLY via MGMT_EV_ADV_MONITOR_DEVICE_FOUND event.
>> > +        */
>> > +       if (discovering) {
>> > +               mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
>> > +
>> > +               if (!hdev->advmon_pend_notify)
>> > +                       return;
>> > +       }
>> > +
>> > +       /* Make sure that the buffer is big enough */
>> > +       advmon_ev_size = ev_size + (sizeof(*advmon_ev) - sizeof(*ev));
>> > +       if (advmon_ev_size > sizeof(buf))
>> > +               return;
>> > +
>> > +       /* ADV_MONITOR_DEVICE_FOUND is similar to DEVICE_FOUND event except
>> > +        * that it also has 'monitor_handle'. Make a copy of DEVICE_FOUND and
>> > +        * store monitor_handle of the matched monitor.
>> > +        */
>> > +       memcpy(&advmon_ev->addr, ev, ev_size);
>> > +
>> > +       hdev->advmon_pend_notify = false;
>> > +
>> > +       list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, list) {
>> > +               if (!bacmp(&dev->bdaddr, &advmon_ev->addr.bdaddr)) {
>> > +                       matched = true;
>> > +
>> > +                       if (!dev->notified) {
>> > +                               advmon_ev->monitor_handle =
>> > +                                               cpu_to_le16(dev->handle);
>> > +
>> > +                               mgmt_event(MGMT_EV_ADV_MONITOR_DEVICE_FOUND,
>> > +                                          hdev, advmon_ev, advmon_ev_size,
>> > +                                          NULL);
>> > +
>> > +                               notified = true;
>> > +                               dev->notified = true;
>> > +                       }
>> > +               }
>> > +
>> > +               if (!dev->notified)
>> > +                       hdev->advmon_pend_notify = true;
>> > +       }
>> > +
>> > +       if (!discovering &&
>> > +           ((matched && !notified) || !msft_monitor_supported(hdev))) {
>> > +               /* Handle 0 indicates that we are not active scanning and this
>> > +                * is a subsequent advertisement report for an already matched
>> > +                * Advertisement Monitor or the controller offloading support
>> > +                * is not available.
>> > +                */
>> > +               advmon_ev->monitor_handle = 0;
>> > +
>> > +               mgmt_event(MGMT_EV_ADV_MONITOR_DEVICE_FOUND, hdev, advmon_ev,
>> > +                          advmon_ev_size, NULL);
>> > +       }
>> > +}
>> > +
>> >  void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>> >                        u8 addr_type, u8 *dev_class, s8 rssi, u32 flags,
>> >                        u8 *eir, u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
>> > @@ -9531,6 +9627,7 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>> >         char buf[512];
>> >         struct mgmt_ev_device_found *ev = (void *)buf;
>> >         size_t ev_size;
>> > +       bool report_device_found = hci_discovery_active(hdev);
>> >
>> >         /* Don't send events for a non-kernel initiated discovery. With
>> >          * LE one exception is if we have pend_le_reports > 0 in which
>> > @@ -9539,11 +9636,10 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>> >         if (!hci_discovery_active(hdev)) {
>> >                 if (link_type == ACL_LINK)
>> >                         return;
>> > -               if (link_type == LE_LINK &&
>> > -                   list_empty(&hdev->pend_le_reports) &&
>> > -                   !hci_is_adv_monitoring(hdev)) {
>> > +               if (link_type == LE_LINK && !list_empty(&hdev->pend_le_reports))
>> > +                       report_device_found = true;
>> > +               else if (!hci_is_adv_monitoring(hdev))
>> >                         return;
>> > -               }
>> >         }
>> >
>> >         if (hdev->discovery.result_filtering) {
>> > @@ -9606,7 +9702,7 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>> >         ev->eir_len = cpu_to_le16(eir_len + scan_rsp_len);
>> >         ev_size = sizeof(*ev) + eir_len + scan_rsp_len;
>> >
>> > -       mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
>> > +       mgmt_adv_monitor_device_found(hdev, ev, ev_size, report_device_found);
>> >  }
>> >
>> >  void mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
>> > diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
>> > index aadabe78baf6..3e2385562d2b 100644
>> > --- a/net/bluetooth/msft.c
>> > +++ b/net/bluetooth/msft.c
>> > @@ -579,8 +579,16 @@ void msft_do_close(struct hci_dev *hdev)
>> >
>> >         hci_dev_lock(hdev);
>> >
>> > -       /* Clear any devices that are being monitored */
>> > +       /* Clear any devices that are being monitored and notify device lost */
>> > +
>> > +       hdev->advmon_pend_notify = false;
>> > +
>> >         list_for_each_entry_safe(dev, tmp_dev, &hdev->monitored_devices, list) {
>> > +               if (dev->notified)
>> > +                       mgmt_adv_monitor_device_lost(hdev, dev->handle,
>> > +                                                    &dev->bdaddr,
>> > +                                                    dev->addr_type);
>> > +
>> >                 list_del(&dev->list);
>> >                 kfree(dev);
>> >         }
>> > @@ -639,6 +647,7 @@ static void msft_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr,
>> >
>> >         INIT_LIST_HEAD(&dev->list);
>> >         list_add(&dev->list, &hdev->monitored_devices);
>> > +       hdev->advmon_pend_notify = true;
>> >  }
>> >
>> >  /* This function requires the caller holds hdev->lock */
>> > @@ -649,6 +658,10 @@ static void msft_device_lost(struct hci_dev *hdev, bdaddr_t *bdaddr,
>> >
>> >         list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, list) {
>> >                 if (dev->handle == mgmt_handle) {
>> > +                       if (dev->notified)
>> > +                               mgmt_adv_monitor_device_lost(hdev, mgmt_handle,
>> > +                                                            bdaddr, addr_type);
>> > +
>> >                         list_del(&dev->list);
>> >                         kfree(dev);
>> >
>> > --
>> > 2.34.0.384.gca35af8252-goog
>> >
>>
>>
>> --
>> Luiz Augusto von Dentz
>
>
> Thanks,
> Manish.



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-12-13 17:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-03  7:16 [PATCH v7 1/2] bluetooth: Handle MSFT Monitor Device Event Manish Mandlik
2021-12-03  7:16 ` [PATCH v7 2/2] bluetooth: Add MGMT Adv Monitor Device Found/Lost events Manish Mandlik
2021-12-03 22:18   ` Luiz Augusto von Dentz
     [not found]     ` <CAGPPCLDreZm2m_ZmymVumu5s1tDeXi7-UGFDFnxd8ZuzW6oVPA@mail.gmail.com>
2021-12-13 17:20       ` Luiz Augusto von Dentz
2021-12-03 22:13 ` [PATCH v7 1/2] bluetooth: Handle MSFT Monitor Device Event Luiz Augusto von Dentz
2021-12-06 13:57 ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).