Re: [PATCH v3 3/6] Bluetooth: Interleave with allowlist scan

2020-09-20 Thread Marcel Holtmann
Hi Howard,

> This patch implements the interleaving between allowlist scan and
> no-filter scan. It'll be used to save power when at least one monitor is
> registered and at least one pending connection or one device to be
> scanned for.
> 
> The durations of the allowlist scan and the no-filter scan are
> controlled by MGMT command: Set Default System Configuration. The
> default values are set randomly for now.
> 
> Signed-off-by: Howard Chung 
> Reviewed-by: Alain Michaud 
> Reviewed-by: Manish Mandlik 
> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - remove 'case 0x001c' in mgmt_config.c
> 
> include/net/bluetooth/hci_core.h |  10 +++
> net/bluetooth/hci_core.c |   4 +
> net/bluetooth/hci_request.c  | 137 +--
> net/bluetooth/mgmt_config.c  |  12 +++
> 4 files changed, 155 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h 
> b/include/net/bluetooth/hci_core.h
> index 9873e1c8cd163..179350f869fdb 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -361,6 +361,8 @@ struct hci_dev {
>   __u8ssp_debug_mode;
>   __u8hw_error_code;
>   __u32   clock;
> + __u16   advmon_allowlist_duration;
> + __u16   advmon_no_filter_duration;
> 
>   __u16   devid_source;
>   __u16   devid_vendor;
> @@ -542,6 +544,14 @@ struct hci_dev {
>   struct delayed_work rpa_expired;
>   bdaddr_trpa;
> 
> + enum {
> + ADV_MONITOR_SCAN_NONE,
> + ADV_MONITOR_SCAN_NO_FILTER,
> + ADV_MONITOR_SCAN_ALLOWLIST
> + } adv_monitor_scan_state;
> +
> + struct delayed_work interleave_adv_monitor_scan;
> +
> #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 f30a1f5950e15..6c8850149265a 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3592,6 +3592,10 @@ struct hci_dev *hci_alloc_dev(void)
>   hdev->cur_adv_instance = 0x00;
>   hdev->adv_instance_timeout = 0;
> 
> + /* The default values will be chosen in the future */
> + hdev->advmon_allowlist_duration = 300;
> + hdev->advmon_no_filter_duration = 500;
> +
>   hdev->sniff_max_interval = 800;
>   hdev->sniff_min_interval = 80;
> 
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index d2b06f5c93804..89443b48d90ce 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -378,6 +378,57 @@ void __hci_req_write_fast_connectable(struct hci_request 
> *req, bool enable)
>   hci_req_add(req, HCI_OP_WRITE_PAGE_SCAN_TYPE, 1, );
> }
> 
> +static void start_interleave_scan(struct hci_dev *hdev)
> +{
> + hdev->adv_monitor_scan_state = ADV_MONITOR_SCAN_NO_FILTER;
> + queue_delayed_work(hdev->req_workqueue,
> +>interleave_adv_monitor_scan, 0);
> +}
> +
> +static bool is_interleave_scanning(struct hci_dev *hdev)
> +{
> + return hdev->adv_monitor_scan_state != ADV_MONITOR_SCAN_NONE;
> +}
> +
> +static void cancel_interleave_scan(struct hci_dev *hdev)
> +{
> + bt_dev_dbg(hdev, "%s cancelling interleave scan", hdev->name);
> +
> + cancel_delayed_work_sync(>interleave_adv_monitor_scan);
> +
> + hdev->adv_monitor_scan_state = ADV_MONITOR_SCAN_NONE;
> +}
> +
> +/* Return true if interleave_scan is running after exiting this function,
> + * otherwise, return false
> + */
> +static bool update_adv_monitor_scan_state(struct hci_dev *hdev)
> +{
> + if (!hci_is_adv_monitoring(hdev) ||
> + (list_empty(>pend_le_conns) &&
> +  list_empty(>pend_le_reports))) {
> + if (is_interleave_scanning(hdev)) {
> + /* If the interleave condition no longer holds, cancel
> +  * the existed interleave scan.
> +  */
> + cancel_interleave_scan(hdev);
> + }
> + return false;
> + }
> +
> + if (!is_interleave_scanning(hdev)) {
> + /* If there is at least one ADV monitors and one pending LE
> +  * connection or one device to be scanned for, we should
> +  * alternate between allowlist scan and one without any filters
> +  * to save power.
> +  */
> + start_interleave_scan(hdev);
> + bt_dev_dbg(hdev, "%s starting interleave scan", hdev->name);
> + }
> +
> + return true;
> +}
> +
> /* This function controls the background scanning based on hdev->pend_le_conns
>  * list. If there are pending LE connection we start the background scanning,
>  * otherwise we stop it.
> @@ -449,9 +500,11 @@ static void __hci_update_background_scan(struct 
> hci_request *req)
>   if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
>   

[PATCH v3 3/6] Bluetooth: Interleave with allowlist scan

2020-09-17 Thread Howard Chung
This patch implements the interleaving between allowlist scan and
no-filter scan. It'll be used to save power when at least one monitor is
registered and at least one pending connection or one device to be
scanned for.

The durations of the allowlist scan and the no-filter scan are
controlled by MGMT command: Set Default System Configuration. The
default values are set randomly for now.

Signed-off-by: Howard Chung 
Reviewed-by: Alain Michaud 
Reviewed-by: Manish Mandlik 
---

(no changes since v2)

Changes in v2:
- remove 'case 0x001c' in mgmt_config.c

 include/net/bluetooth/hci_core.h |  10 +++
 net/bluetooth/hci_core.c |   4 +
 net/bluetooth/hci_request.c  | 137 +--
 net/bluetooth/mgmt_config.c  |  12 +++
 4 files changed, 155 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 9873e1c8cd163..179350f869fdb 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -361,6 +361,8 @@ struct hci_dev {
__u8ssp_debug_mode;
__u8hw_error_code;
__u32   clock;
+   __u16   advmon_allowlist_duration;
+   __u16   advmon_no_filter_duration;
 
__u16   devid_source;
__u16   devid_vendor;
@@ -542,6 +544,14 @@ struct hci_dev {
struct delayed_work rpa_expired;
bdaddr_trpa;
 
+   enum {
+   ADV_MONITOR_SCAN_NONE,
+   ADV_MONITOR_SCAN_NO_FILTER,
+   ADV_MONITOR_SCAN_ALLOWLIST
+   } adv_monitor_scan_state;
+
+   struct delayed_work interleave_adv_monitor_scan;
+
 #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 f30a1f5950e15..6c8850149265a 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3592,6 +3592,10 @@ struct hci_dev *hci_alloc_dev(void)
hdev->cur_adv_instance = 0x00;
hdev->adv_instance_timeout = 0;
 
+   /* The default values will be chosen in the future */
+   hdev->advmon_allowlist_duration = 300;
+   hdev->advmon_no_filter_duration = 500;
+
hdev->sniff_max_interval = 800;
hdev->sniff_min_interval = 80;
 
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index d2b06f5c93804..89443b48d90ce 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -378,6 +378,57 @@ void __hci_req_write_fast_connectable(struct hci_request 
*req, bool enable)
hci_req_add(req, HCI_OP_WRITE_PAGE_SCAN_TYPE, 1, );
 }
 
+static void start_interleave_scan(struct hci_dev *hdev)
+{
+   hdev->adv_monitor_scan_state = ADV_MONITOR_SCAN_NO_FILTER;
+   queue_delayed_work(hdev->req_workqueue,
+  >interleave_adv_monitor_scan, 0);
+}
+
+static bool is_interleave_scanning(struct hci_dev *hdev)
+{
+   return hdev->adv_monitor_scan_state != ADV_MONITOR_SCAN_NONE;
+}
+
+static void cancel_interleave_scan(struct hci_dev *hdev)
+{
+   bt_dev_dbg(hdev, "%s cancelling interleave scan", hdev->name);
+
+   cancel_delayed_work_sync(>interleave_adv_monitor_scan);
+
+   hdev->adv_monitor_scan_state = ADV_MONITOR_SCAN_NONE;
+}
+
+/* Return true if interleave_scan is running after exiting this function,
+ * otherwise, return false
+ */
+static bool update_adv_monitor_scan_state(struct hci_dev *hdev)
+{
+   if (!hci_is_adv_monitoring(hdev) ||
+   (list_empty(>pend_le_conns) &&
+list_empty(>pend_le_reports))) {
+   if (is_interleave_scanning(hdev)) {
+   /* If the interleave condition no longer holds, cancel
+* the existed interleave scan.
+*/
+   cancel_interleave_scan(hdev);
+   }
+   return false;
+   }
+
+   if (!is_interleave_scanning(hdev)) {
+   /* If there is at least one ADV monitors and one pending LE
+* connection or one device to be scanned for, we should
+* alternate between allowlist scan and one without any filters
+* to save power.
+*/
+   start_interleave_scan(hdev);
+   bt_dev_dbg(hdev, "%s starting interleave scan", hdev->name);
+   }
+
+   return true;
+}
+
 /* This function controls the background scanning based on hdev->pend_le_conns
  * list. If there are pending LE connection we start the background scanning,
  * otherwise we stop it.
@@ -449,9 +500,11 @@ static void __hci_update_background_scan(struct 
hci_request *req)
if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
hci_req_add_le_scan_disable(req, false);
 
-   hci_req_add_le_passive_scan(req);
-
-   BT_DBG("%s starting background scanning", hdev->name);
+