2021-11-21 19:12:34

by Manish Mandlik

[permalink] [raw]
Subject: [PATCH v6 2/2] bluetooth: Add MGMT Adv Monitor Device Found/Lost events

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 <[email protected]>
Reviewed-by: Miao-chen Chou <[email protected]>

---

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 | 100 ++++++++++++++++++++++++++++++-
net/bluetooth/msft.c | 15 ++++-
4 files changed, 132 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 6734b394c6e7..2aabd6f62f51 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -598,6 +598,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;
@@ -1844,6 +1845,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 23a0524061b7..4b85f93b8a77 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -1103,3 +1103,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 f8f74d344297..01f74e4feb97 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,78 @@ 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;
+
+ /* 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)
@@ -9606,7 +9680,31 @@ 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);
+ /* 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 (hci_discovery_active(hdev) ||
+ (link_type == LE_LINK && !list_empty(&hdev->pend_le_reports))) {
+ mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
+
+ if (hdev->advmon_pend_notify)
+ mgmt_adv_monitor_device_found(hdev, ev, ev_size, true);
+ } else {
+ mgmt_adv_monitor_device_found(hdev, ev, ev_size, false);
+ }
}

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.rc2.393.gf8c9666880-goog



2021-11-22 11:01:54

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v6 2/2] bluetooth: Add MGMT Adv Monitor Device Found/Lost events

Hi Manish,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on next-20211118]
[cannot apply to bluetooth/master v5.16-rc2]
[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-Handle-MSFT-Monitor-Device-Event/20211122-031347
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: hexagon-randconfig-r005-20211122 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c133fb321f7ca6083ce15b6aa5bf89de6600e649)
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/b61f97a7820d3965cf6e5bbf719449c667bf1925
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Manish-Mandlik/bluetooth-Handle-MSFT-Monitor-Device-Event/20211122-031347
git checkout b61f97a7820d3965cf6e5bbf719449c667bf1925
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=hexagon

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

All warnings (new ones prefixed by >>):

>> net/bluetooth/mgmt.c:9601:6: warning: stack frame size (1120) exceeds limit (1024) in 'mgmt_device_found' [-Wframe-larger-than]
void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
^
1 warning generated.


vim +/mgmt_device_found +9601 net/bluetooth/mgmt.c

b61f97a7820d39 Manish Mandlik 2021-11-21 9600
48f86b7f267335 Jakub Pawlowski 2015-03-04 @9601 void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
48f86b7f267335 Jakub Pawlowski 2015-03-04 9602 u8 addr_type, u8 *dev_class, s8 rssi, u32 flags,
48f86b7f267335 Jakub Pawlowski 2015-03-04 9603 u8 *eir, u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
48f86b7f267335 Jakub Pawlowski 2015-03-04 9604 {
48f86b7f267335 Jakub Pawlowski 2015-03-04 9605 char buf[512];
48f86b7f267335 Jakub Pawlowski 2015-03-04 9606 struct mgmt_ev_device_found *ev = (void *)buf;
48f86b7f267335 Jakub Pawlowski 2015-03-04 9607 size_t ev_size;
48f86b7f267335 Jakub Pawlowski 2015-03-04 9608
48f86b7f267335 Jakub Pawlowski 2015-03-04 9609 /* Don't send events for a non-kernel initiated discovery. With
48f86b7f267335 Jakub Pawlowski 2015-03-04 9610 * LE one exception is if we have pend_le_reports > 0 in which
48f86b7f267335 Jakub Pawlowski 2015-03-04 9611 * case we're doing passive scanning and want these events.
48f86b7f267335 Jakub Pawlowski 2015-03-04 9612 */
48f86b7f267335 Jakub Pawlowski 2015-03-04 9613 if (!hci_discovery_active(hdev)) {
48f86b7f267335 Jakub Pawlowski 2015-03-04 9614 if (link_type == ACL_LINK)
48f86b7f267335 Jakub Pawlowski 2015-03-04 9615 return;
8208f5a9d435e5 Miao-chen Chou 2020-06-17 9616 if (link_type == LE_LINK &&
8208f5a9d435e5 Miao-chen Chou 2020-06-17 9617 list_empty(&hdev->pend_le_reports) &&
8208f5a9d435e5 Miao-chen Chou 2020-06-17 9618 !hci_is_adv_monitoring(hdev)) {
48f86b7f267335 Jakub Pawlowski 2015-03-04 9619 return;
48f86b7f267335 Jakub Pawlowski 2015-03-04 9620 }
8208f5a9d435e5 Miao-chen Chou 2020-06-17 9621 }
48f86b7f267335 Jakub Pawlowski 2015-03-04 9622
82f8b651a94d5c Jakub Pawlowski 2015-03-04 9623 if (hdev->discovery.result_filtering) {
48f86b7f267335 Jakub Pawlowski 2015-03-04 9624 /* We are using service discovery */
48f86b7f267335 Jakub Pawlowski 2015-03-04 9625 if (!is_filter_match(hdev, rssi, eir, eir_len, scan_rsp,
48f86b7f267335 Jakub Pawlowski 2015-03-04 9626 scan_rsp_len))
48f86b7f267335 Jakub Pawlowski 2015-03-04 9627 return;
48f86b7f267335 Jakub Pawlowski 2015-03-04 9628 }
48f86b7f267335 Jakub Pawlowski 2015-03-04 9629
78b781ca0d3519 Johan Hedberg 2016-01-05 9630 if (hdev->discovery.limited) {
78b781ca0d3519 Johan Hedberg 2016-01-05 9631 /* Check for limited discoverable bit */
78b781ca0d3519 Johan Hedberg 2016-01-05 9632 if (dev_class) {
78b781ca0d3519 Johan Hedberg 2016-01-05 9633 if (!(dev_class[1] & 0x20))
78b781ca0d3519 Johan Hedberg 2016-01-05 9634 return;
78b781ca0d3519 Johan Hedberg 2016-01-05 9635 } else {
78b781ca0d3519 Johan Hedberg 2016-01-05 9636 u8 *flags = eir_get_data(eir, eir_len, EIR_FLAGS, NULL);
78b781ca0d3519 Johan Hedberg 2016-01-05 9637 if (!flags || !(flags[0] & LE_AD_LIMITED))
78b781ca0d3519 Johan Hedberg 2016-01-05 9638 return;
78b781ca0d3519 Johan Hedberg 2016-01-05 9639 }
78b781ca0d3519 Johan Hedberg 2016-01-05 9640 }
78b781ca0d3519 Johan Hedberg 2016-01-05 9641
48f86b7f267335 Jakub Pawlowski 2015-03-04 9642 /* Make sure that the buffer is big enough. The 5 extra bytes
48f86b7f267335 Jakub Pawlowski 2015-03-04 9643 * are for the potential CoD field.
48f86b7f267335 Jakub Pawlowski 2015-03-04 9644 */
48f86b7f267335 Jakub Pawlowski 2015-03-04 9645 if (sizeof(*ev) + eir_len + scan_rsp_len + 5 > sizeof(buf))
4b0e0ceddf085a Jakub Pawlowski 2015-02-01 9646 return;
4b0e0ceddf085a Jakub Pawlowski 2015-02-01 9647
48f86b7f267335 Jakub Pawlowski 2015-03-04 9648 memset(buf, 0, sizeof(buf));
48f86b7f267335 Jakub Pawlowski 2015-03-04 9649
48f86b7f267335 Jakub Pawlowski 2015-03-04 9650 /* In case of device discovery with BR/EDR devices (pre 1.2), the
48f86b7f267335 Jakub Pawlowski 2015-03-04 9651 * RSSI value was reported as 0 when not available. This behavior
48f86b7f267335 Jakub Pawlowski 2015-03-04 9652 * is kept when using device discovery. This is required for full
48f86b7f267335 Jakub Pawlowski 2015-03-04 9653 * backwards compatibility with the API.
48f86b7f267335 Jakub Pawlowski 2015-03-04 9654 *
48f86b7f267335 Jakub Pawlowski 2015-03-04 9655 * However when using service discovery, the value 127 will be
48f86b7f267335 Jakub Pawlowski 2015-03-04 9656 * returned when the RSSI is not available.
48f86b7f267335 Jakub Pawlowski 2015-03-04 9657 */
48f86b7f267335 Jakub Pawlowski 2015-03-04 9658 if (rssi == HCI_RSSI_INVALID && !hdev->discovery.report_invalid_rssi &&
48f86b7f267335 Jakub Pawlowski 2015-03-04 9659 link_type == ACL_LINK)
48f86b7f267335 Jakub Pawlowski 2015-03-04 9660 rssi = 0;
48f86b7f267335 Jakub Pawlowski 2015-03-04 9661
48f86b7f267335 Jakub Pawlowski 2015-03-04 9662 bacpy(&ev->addr.bdaddr, bdaddr);
48f86b7f267335 Jakub Pawlowski 2015-03-04 9663 ev->addr.type = link_to_bdaddr(link_type, addr_type);
48f86b7f267335 Jakub Pawlowski 2015-03-04 9664 ev->rssi = rssi;
48f86b7f267335 Jakub Pawlowski 2015-03-04 9665 ev->flags = cpu_to_le32(flags);
48f86b7f267335 Jakub Pawlowski 2015-03-04 9666
48f86b7f267335 Jakub Pawlowski 2015-03-04 9667 if (eir_len > 0)
48f86b7f267335 Jakub Pawlowski 2015-03-04 9668 /* Copy EIR or advertising data into event */
48f86b7f267335 Jakub Pawlowski 2015-03-04 9669 memcpy(ev->eir, eir, eir_len);
48f86b7f267335 Jakub Pawlowski 2015-03-04 9670
0d3b7f64c84d53 Johan Hedberg 2016-01-05 9671 if (dev_class && !eir_get_data(ev->eir, eir_len, EIR_CLASS_OF_DEV,
0d3b7f64c84d53 Johan Hedberg 2016-01-05 9672 NULL))
48f86b7f267335 Jakub Pawlowski 2015-03-04 9673 eir_len = eir_append_data(ev->eir, eir_len, EIR_CLASS_OF_DEV,
48f86b7f267335 Jakub Pawlowski 2015-03-04 9674 dev_class, 3);
48f86b7f267335 Jakub Pawlowski 2015-03-04 9675
48f86b7f267335 Jakub Pawlowski 2015-03-04 9676 if (scan_rsp_len > 0)
48f86b7f267335 Jakub Pawlowski 2015-03-04 9677 /* Append scan response data to event */
48f86b7f267335 Jakub Pawlowski 2015-03-04 9678 memcpy(ev->eir + eir_len, scan_rsp, scan_rsp_len);
48f86b7f267335 Jakub Pawlowski 2015-03-04 9679
5d2e9fadf43e87 Johan Hedberg 2014-03-25 9680 ev->eir_len = cpu_to_le16(eir_len + scan_rsp_len);
5d2e9fadf43e87 Johan Hedberg 2014-03-25 9681 ev_size = sizeof(*ev) + eir_len + scan_rsp_len;
f8523598ee608a Andre Guedes 2011-09-09 9682
b61f97a7820d39 Manish Mandlik 2021-11-21 9683 /* We have received the Advertisement Report because:
b61f97a7820d39 Manish Mandlik 2021-11-21 9684 * 1. the kernel has initiated active discovery
b61f97a7820d39 Manish Mandlik 2021-11-21 9685 * 2. if not, we have pend_le_reports > 0 in which case we are doing
b61f97a7820d39 Manish Mandlik 2021-11-21 9686 * passive scanning
b61f97a7820d39 Manish Mandlik 2021-11-21 9687 * 3. if none of the above is true, we have one or more active
b61f97a7820d39 Manish Mandlik 2021-11-21 9688 * Advertisement Monitor
b61f97a7820d39 Manish Mandlik 2021-11-21 9689 *
b61f97a7820d39 Manish Mandlik 2021-11-21 9690 * For case 1 and 2, report all advertisements via MGMT_EV_DEVICE_FOUND
b61f97a7820d39 Manish Mandlik 2021-11-21 9691 * and report ONLY one advertisement per device for the matched Monitor
b61f97a7820d39 Manish Mandlik 2021-11-21 9692 * via MGMT_EV_ADV_MONITOR_DEVICE_FOUND event.
b61f97a7820d39 Manish Mandlik 2021-11-21 9693 *
b61f97a7820d39 Manish Mandlik 2021-11-21 9694 * For case 3, since we are not active scanning and all advertisements
b61f97a7820d39 Manish Mandlik 2021-11-21 9695 * received are due to a matched Advertisement Monitor, report all
b61f97a7820d39 Manish Mandlik 2021-11-21 9696 * advertisements ONLY via MGMT_EV_ADV_MONITOR_DEVICE_FOUND event.
b61f97a7820d39 Manish Mandlik 2021-11-21 9697 */
b61f97a7820d39 Manish Mandlik 2021-11-21 9698
b61f97a7820d39 Manish Mandlik 2021-11-21 9699 if (hci_discovery_active(hdev) ||
b61f97a7820d39 Manish Mandlik 2021-11-21 9700 (link_type == LE_LINK && !list_empty(&hdev->pend_le_reports))) {
901801b9a420e5 Marcel Holtmann 2013-10-06 9701 mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
b61f97a7820d39 Manish Mandlik 2021-11-21 9702
b61f97a7820d39 Manish Mandlik 2021-11-21 9703 if (hdev->advmon_pend_notify)
b61f97a7820d39 Manish Mandlik 2021-11-21 9704 mgmt_adv_monitor_device_found(hdev, ev, ev_size, true);
b61f97a7820d39 Manish Mandlik 2021-11-21 9705 } else {
b61f97a7820d39 Manish Mandlik 2021-11-21 9706 mgmt_adv_monitor_device_found(hdev, ev, ev_size, false);
b61f97a7820d39 Manish Mandlik 2021-11-21 9707 }
e17acd40f6006d Johan Hedberg 2011-03-30 9708 }
a88a9652d25a63 Johan Hedberg 2011-03-30 9709

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


Attachments:
(No filename) (10.63 kB)
.config.gz (28.73 kB)
Download all attachments

2021-11-29 17:42:56

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v6 2/2] bluetooth: Add MGMT Adv Monitor Device Found/Lost events

Hi Manish,

> 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 <[email protected]>
> Reviewed-by: Miao-chen Chou <[email protected]>
>
> ---
>
> 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 | 100 ++++++++++++++++++++++++++++++-
> net/bluetooth/msft.c | 15 ++++-
> 4 files changed, 132 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 6734b394c6e7..2aabd6f62f51 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -598,6 +598,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;
> @@ -1844,6 +1845,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 23a0524061b7..4b85f93b8a77 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -1103,3 +1103,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 f8f74d344297..01f74e4feb97 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,78 @@ 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;
> +
> + /* 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)
> @@ -9606,7 +9680,31 @@ 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);
> + /* 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 (hci_discovery_active(hdev) ||
> + (link_type == LE_LINK && !list_empty(&hdev->pend_le_reports))) {
> + mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
> +
> + if (hdev->advmon_pend_notify)
> + mgmt_adv_monitor_device_found(hdev, ev, ev_size, true);
> + } else {
> + mgmt_adv_monitor_device_found(hdev, ev, ev_size, false);
> + }
> }

so you are breaking the stack-frame-size now. You might need to re-design the general device found event handling to fit into a 1k stack frame size.

Regards

Marcel