diff mbox series

[v7,2/2] bluetooth: Add MGMT Adv Monitor Device Found/Lost events

Message ID 20211202231123.v7.2.I9eda306e4c542010535dc49b5488946af592795e@changeid (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series [v7,1/2] bluetooth: Handle MSFT Monitor Device Event | expand

Checks

Context Check Description
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 37 this patch: 37
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 37 this patch: 37
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 205 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Manish Mandlik Dec. 3, 2021, 7:16 a.m. UTC
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(-)

Comments

Luiz Augusto von Dentz Dec. 3, 2021, 10:18 p.m. UTC | #1
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 Dec. 13, 2021, 5:20 p.m. UTC | #2
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.
diff mbox series

Patch

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);