Re: [PATCH v3 2/5] Bluetooth: advmon offload MSFT add monitor
Hi Archie,
> Enables advertising monitor offloading to the controller, if MSFT
> extension is supported. The kernel won't adjust the monitor parameters
> to match what the controller supports - that is the user space's
> responsibility.
>
> This patch only manages the addition of monitors. Monitor removal is
> going to be handled by another patch.
>
> Signed-off-by: Archie Pusaka
> Reviewed-by: Manish Mandlik
> Reviewed-by: Miao-chen Chou
> Reviewed-by: Yun-Hao Chung
>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> * Also implement the new MGMT opcode and merge the functionality with
> the old one.
>
> include/net/bluetooth/hci_core.h | 17 ++-
> net/bluetooth/hci_core.c | 54 +++--
> net/bluetooth/mgmt.c | 144 ++
> net/bluetooth/msft.c | 201 ++-
> net/bluetooth/msft.h | 12 ++
> 5 files changed, 367 insertions(+), 61 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h
> b/include/net/bluetooth/hci_core.h
> index 8b7cf3620938..879d1e38ce96 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -261,13 +261,20 @@ struct adv_rssi_thresholds {
> struct adv_monitor {
> struct list_head patterns;
> struct adv_rssi_thresholds rssi;
> - boolactive;
> __u16 handle;
> +
> + enum {
> + ADV_MONITOR_STATE_NOT_REGISTERED,
> + ADV_MONITOR_STATE_REGISTERED,
> + ADV_MONITOR_STATE_OFFLOADED
> + } state;
> };
>
> #define HCI_MIN_ADV_MONITOR_HANDLE1
> -#define HCI_MAX_ADV_MONITOR_NUM_HANDLES 32
> +#define HCI_MAX_ADV_MONITOR_NUM_HANDLES 32
> #define HCI_MAX_ADV_MONITOR_NUM_PATTERNS 16
> +#define HCI_ADV_MONITOR_EXT_NONE 1
> +#define HCI_ADV_MONITOR_EXT_MSFT 2
>
> #define HCI_MAX_SHORT_NAME_LENGTH 10
>
> @@ -1326,9 +1333,12 @@ void hci_adv_instances_set_rpa_expired(struct hci_dev
> *hdev, bool rpa_expired);
>
> void hci_adv_monitors_clear(struct hci_dev *hdev);
> void hci_free_adv_monitor(struct adv_monitor *monitor);
> -int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
> +int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
> +bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
> + int *err);
> int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
> bool hci_is_adv_monitoring(struct hci_dev *hdev);
> +int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
>
> void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
>
> @@ -1804,6 +1814,7 @@ void mgmt_advertising_added(struct sock *sk, struct
> hci_dev *hdev,
> void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
> u8 instance);
> 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);
>
> u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
> u16 to_multiplier);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9d2c9a1c552f..fa13e35f775d 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3070,27 +3070,55 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
> kfree(monitor);
> }
>
> -/* This function requires the caller holds hdev->lock */
> -int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
> +int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
> +{
> + return mgmt_add_adv_patterns_monitor_complete(hdev, status);
> +}
> +
> +/* Assigns handle to a monitor, and if offloading is supported and power is
> on,
> + * also attempts to forward the request to the controller.
> + * Returns true if request is forwarded (result is pending), false otherwise.
> + * This function requires the caller holds hdev->lock.
> + */
> +bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
> + int *err)
> {
> int min, max, handle;
>
> - if (!monitor)
> - return -EINVAL;
> + *err = 0;
> +
> + if (!monitor) {
> + *err = -EINVAL;
> + return false;
> + }
>
> min = HCI_MIN_ADV_MONITOR_HANDLE;
> max = HCI_MIN_ADV_MONITOR_HANDLE + HCI_MAX_ADV_MONITOR_NUM_HANDLES;
> handle = idr_alloc(>adv_monitors_idr, monitor, min, max,
> GFP_KERNEL);
> - if (handle < 0)
> - return handle;
> + if (handle < 0) {
> + *err = handle;
> + return false;
> + }
>
> - hdev->adv_monitors_cnt++;
> monitor->handle = handle;
>
> - hci_update_background_scan(hdev);
> + if (!hdev_is_powered(hdev))
> + return false;
>
> - return 0;
> + switch
[PATCH v3 2/5] Bluetooth: advmon offload MSFT add monitor
From: Archie Pusaka
Enables advertising monitor offloading to the controller, if MSFT
extension is supported. The kernel won't adjust the monitor parameters
to match what the controller supports - that is the user space's
responsibility.
This patch only manages the addition of monitors. Monitor removal is
going to be handled by another patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Manish Mandlik
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v2)
Changes in v2:
* Also implement the new MGMT opcode and merge the functionality with
the old one.
include/net/bluetooth/hci_core.h | 17 ++-
net/bluetooth/hci_core.c | 54 +++--
net/bluetooth/mgmt.c | 144 ++
net/bluetooth/msft.c | 201 ++-
net/bluetooth/msft.h | 12 ++
5 files changed, 367 insertions(+), 61 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8b7cf3620938..879d1e38ce96 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -261,13 +261,20 @@ struct adv_rssi_thresholds {
struct adv_monitor {
struct list_head patterns;
struct adv_rssi_thresholds rssi;
- boolactive;
__u16 handle;
+
+ enum {
+ ADV_MONITOR_STATE_NOT_REGISTERED,
+ ADV_MONITOR_STATE_REGISTERED,
+ ADV_MONITOR_STATE_OFFLOADED
+ } state;
};
#define HCI_MIN_ADV_MONITOR_HANDLE 1
-#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
+#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
#define HCI_MAX_ADV_MONITOR_NUM_PATTERNS 16
+#define HCI_ADV_MONITOR_EXT_NONE 1
+#define HCI_ADV_MONITOR_EXT_MSFT 2
#define HCI_MAX_SHORT_NAME_LENGTH 10
@@ -1326,9 +1333,12 @@ void hci_adv_instances_set_rpa_expired(struct hci_dev
*hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
void hci_free_adv_monitor(struct adv_monitor *monitor);
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+ int *err);
int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
+int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
@@ -1804,6 +1814,7 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
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);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9d2c9a1c552f..fa13e35f775d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3070,27 +3070,55 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
kfree(monitor);
}
-/* This function requires the caller holds hdev->lock */
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_add_adv_patterns_monitor_complete(hdev, status);
+}
+
+/* Assigns handle to a monitor, and if offloading is supported and power is on,
+ * also attempts to forward the request to the controller.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+int *err)
{
int min, max, handle;
- if (!monitor)
- return -EINVAL;
+ *err = 0;
+
+ if (!monitor) {
+ *err = -EINVAL;
+ return false;
+ }
min = HCI_MIN_ADV_MONITOR_HANDLE;
max = HCI_MIN_ADV_MONITOR_HANDLE + HCI_MAX_ADV_MONITOR_NUM_HANDLES;
handle = idr_alloc(>adv_monitors_idr, monitor, min, max,
GFP_KERNEL);
- if (handle < 0)
- return handle;
+ if (handle < 0) {
+ *err = handle;
+ return false;
+ }
- hdev->adv_monitors_cnt++;
monitor->handle = handle;
- hci_update_background_scan(hdev);
+ if (!hdev_is_powered(hdev))
+ return false;
- return 0;
+ switch (hci_get_adv_monitor_offload_ext(hdev)) {
+ case HCI_ADV_MONITOR_EXT_NONE:
+ hci_update_background_scan(hdev);
+ BT_DBG("%s add
