* [PATCH v6 1/4] Bluetooth: Interleave with allowlist scan
@ 2020-09-28 7:41 Howard Chung
2020-09-28 7:41 ` [PATCH v6 2/4] Bluetooth: Handle system suspend resume case Howard Chung
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Howard Chung @ 2020-09-28 7:41 UTC (permalink / raw)
To: linux-bluetooth, marcel, luiz.dentz
Cc: alainm, mcchou, mmandlik, Howard Chung, Manish Mandlik,
David S. Miller, Jakub Kicinski, Johan Hedberg, linux-kernel,
netdev
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 <howardchung@google.com>
Reviewed-by: Alain Michaud <alainm@chromium.org>
Reviewed-by: Manish Mandlik <mmandlik@chromium.org>
---
Changes in v6:
- Change the type of enable_advmon_interleave_scan to u8
Changes in v5:
- Rename 'adv_monitor' from many functions/variables
- Move __hci_update_interleaved_scan into hci_req_add_le_passive_scan
- Update the logic of update_adv_monitor_scan_state
Changes in v4:
- Rebase to bluetooth-next/master (previous 2 patches are applied)
- Fix over 80 chars limit in mgmt_config.c
- Set EnableAdvMonInterleaveScan default to Disable
Changes in v3:
- Remove 'Bluez' prefix
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 | 133 +++++++++++++++++++++++++++++--
net/bluetooth/mgmt_config.c | 10 +++
4 files changed, 150 insertions(+), 7 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 9873e1c8cd163..cfede18709d8f 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -361,6 +361,8 @@ struct hci_dev {
__u8 ssp_debug_mode;
__u8 hw_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_t rpa;
+ enum {
+ INTERLEAVE_SCAN_NONE,
+ INTERLEAVE_SCAN_NO_FILTER,
+ INTERLEAVE_SCAN_ALLOWLIST
+ } interleave_scan_state;
+
+ struct delayed_work interleave_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..ba3016cc0b573 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -378,6 +378,56 @@ void __hci_req_write_fast_connectable(struct hci_request *req, bool enable)
hci_req_add(req, HCI_OP_WRITE_PAGE_SCAN_TYPE, 1, &type);
}
+static void start_interleave_scan(struct hci_dev *hdev)
+{
+ hdev->interleave_scan_state = INTERLEAVE_SCAN_NO_FILTER;
+ queue_delayed_work(hdev->req_workqueue,
+ &hdev->interleave_scan, 0);
+}
+
+static bool is_interleave_scanning(struct hci_dev *hdev)
+{
+ return hdev->interleave_scan_state != INTERLEAVE_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(&hdev->interleave_scan);
+
+ hdev->interleave_scan_state = INTERLEAVE_SCAN_NONE;
+}
+
+/* Return true if interleave_scan wasn't started until exiting this function,
+ * otherwise, return false
+ */
+static bool __hci_update_interleaved_scan(struct hci_dev *hdev)
+{
+ if (hci_is_adv_monitoring(hdev) &&
+ !(list_empty(&hdev->pend_le_conns) &&
+ list_empty(&hdev->pend_le_reports))) {
+ 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;
+ }
+ } else if (is_interleave_scanning(hdev)) {
+ /* If the interleave condition no longer holds, cancel
+ * the existed interleave scan.
+ */
+ cancel_interleave_scan(hdev);
+ }
+
+ return false;
+}
+
/* 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.
@@ -450,8 +500,8 @@ static void __hci_update_background_scan(struct hci_request *req)
hci_req_add_le_scan_disable(req, false);
hci_req_add_le_passive_scan(req);
-
- BT_DBG("%s starting background scanning", hdev->name);
+ bt_dev_dbg(hdev, "%s starting background scanning",
+ hdev->name);
}
}
@@ -844,12 +894,17 @@ static u8 update_white_list(struct hci_request *req)
return 0x00;
}
- /* Once the controller offloading of advertisement monitor is in place,
- * the if condition should include the support of MSFT extension
- * support. If suspend is ongoing, whitelist should be the default to
- * prevent waking by random advertisements.
+ /* Use the allowlist unless the following conditions are all true:
+ * - We are not currently suspending
+ * - There are 1 or more ADV monitors registered
+ * - Interleaved scanning is not currently using the allowlist
+ *
+ * Once the controller offloading of advertisement monitor is in place,
+ * the above condition should include the support of MSFT extension
+ * support.
*/
- if (!idr_is_empty(&hdev->adv_monitors_idr) && !hdev->suspended)
+ if (!idr_is_empty(&hdev->adv_monitors_idr) && !hdev->suspended &&
+ hdev->interleave_scan_state != INTERLEAVE_SCAN_ALLOWLIST)
return 0x00;
/* Select filter policy to use white list */
@@ -1002,6 +1057,9 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
&own_addr_type))
return;
+ if (__hci_update_interleaved_scan(hdev))
+ return;
+
/* Adding or removing entries from the white list must
* happen before enabling scanning. The controller does
* not allow white list modification while scanning.
@@ -1871,6 +1929,64 @@ static void adv_timeout_expire(struct work_struct *work)
hci_dev_unlock(hdev);
}
+static int hci_req_add_le_interleaved_scan(struct hci_request *req,
+ unsigned long opt)
+{
+ struct hci_dev *hdev = req->hdev;
+ int ret = 0;
+
+ hci_dev_lock(hdev);
+
+ if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
+ hci_req_add_le_scan_disable(req, false);
+ hci_req_add_le_passive_scan(req);
+
+ switch (hdev->interleave_scan_state) {
+ case INTERLEAVE_SCAN_ALLOWLIST:
+ bt_dev_dbg(hdev, "next state: allowlist");
+ hdev->interleave_scan_state = INTERLEAVE_SCAN_NO_FILTER;
+ break;
+ case INTERLEAVE_SCAN_NO_FILTER:
+ bt_dev_dbg(hdev, "next state: no filter");
+ hdev->interleave_scan_state = INTERLEAVE_SCAN_ALLOWLIST;
+ break;
+ case INTERLEAVE_SCAN_NONE:
+ default:
+ BT_ERR("unexpected error");
+ ret = -1;
+ }
+
+ hci_dev_unlock(hdev);
+
+ return ret;
+}
+
+static void interleave_scan_work(struct work_struct *work)
+{
+ struct hci_dev *hdev = container_of(work, struct hci_dev,
+ interleave_scan.work);
+ u8 status;
+ unsigned long timeout;
+
+ if (hdev->interleave_scan_state == INTERLEAVE_SCAN_ALLOWLIST) {
+ timeout = msecs_to_jiffies(hdev->advmon_allowlist_duration);
+ } else if (hdev->interleave_scan_state == INTERLEAVE_SCAN_NO_FILTER) {
+ timeout = msecs_to_jiffies(hdev->advmon_no_filter_duration);
+ } else {
+ bt_dev_err(hdev, "unexpected error");
+ return;
+ }
+
+ hci_req_sync(hdev, hci_req_add_le_interleaved_scan, 0,
+ HCI_CMD_TIMEOUT, &status);
+
+ /* Don't continue interleaving if it was canceled */
+ if (is_interleave_scanning(hdev)) {
+ queue_delayed_work(hdev->req_workqueue,
+ &hdev->interleave_scan, timeout);
+ }
+}
+
int hci_get_random_address(struct hci_dev *hdev, bool require_privacy,
bool use_rpa, struct adv_info *adv_instance,
u8 *own_addr_type, bdaddr_t *rand_addr)
@@ -3292,6 +3408,7 @@ void hci_request_setup(struct hci_dev *hdev)
INIT_DELAYED_WORK(&hdev->le_scan_disable, le_scan_disable_work);
INIT_DELAYED_WORK(&hdev->le_scan_restart, le_scan_restart_work);
INIT_DELAYED_WORK(&hdev->adv_instance_expire, adv_timeout_expire);
+ INIT_DELAYED_WORK(&hdev->interleave_scan, interleave_scan_work);
}
void hci_request_cancel_all(struct hci_dev *hdev)
@@ -3311,4 +3428,6 @@ void hci_request_cancel_all(struct hci_dev *hdev)
cancel_delayed_work_sync(&hdev->adv_instance_expire);
hdev->adv_instance_timeout = 0;
}
+
+ cancel_interleave_scan(hdev);
}
diff --git a/net/bluetooth/mgmt_config.c b/net/bluetooth/mgmt_config.c
index b30b571f8caf8..2d3ad288c78ac 100644
--- a/net/bluetooth/mgmt_config.c
+++ b/net/bluetooth/mgmt_config.c
@@ -67,6 +67,8 @@ int read_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
HDEV_PARAM_U16(0x001a, le_supv_timeout),
HDEV_PARAM_U16_JIFFIES_TO_MSECS(0x001b,
def_le_autoconnect_timeout),
+ HDEV_PARAM_U16(0x001d, advmon_allowlist_duration),
+ HDEV_PARAM_U16(0x001e, advmon_no_filter_duration),
};
struct mgmt_rp_read_def_system_config *rp = (void *)params;
@@ -138,6 +140,8 @@ int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
case 0x0019:
case 0x001a:
case 0x001b:
+ case 0x001d:
+ case 0x001e:
if (len != sizeof(u16)) {
bt_dev_warn(hdev, "invalid length %d, exp %zu for type %d",
len, sizeof(u16), type);
@@ -251,6 +255,12 @@ int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
hdev->def_le_autoconnect_timeout =
msecs_to_jiffies(TLV_GET_LE16(buffer));
break;
+ case 0x0001d:
+ hdev->advmon_allowlist_duration = TLV_GET_LE16(buffer);
+ break;
+ case 0x0001e:
+ hdev->advmon_no_filter_duration = TLV_GET_LE16(buffer);
+ break;
default:
bt_dev_warn(hdev, "unsupported parameter %u", type);
break;
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v6 2/4] Bluetooth: Handle system suspend resume case
2020-09-28 7:41 [PATCH v6 1/4] Bluetooth: Interleave with allowlist scan Howard Chung
@ 2020-09-28 7:41 ` Howard Chung
2020-09-28 7:41 ` [PATCH v6 3/4] Bluetooth: Handle active scan case Howard Chung
2020-09-28 7:41 ` [PATCH v6 4/4] Bluetooth: Add toggle to switch off interleave scan Howard Chung
2 siblings, 0 replies; 6+ messages in thread
From: Howard Chung @ 2020-09-28 7:41 UTC (permalink / raw)
To: linux-bluetooth, marcel, luiz.dentz
Cc: alainm, mcchou, mmandlik, Howard Chung, Manish Mandlik,
Abhishek Pandit-Subedi, David S. Miller, Jakub Kicinski,
Johan Hedberg, linux-kernel, netdev
This patch adds code to handle the system suspension during interleave
scan. The interleave scan will be canceled when the system is going to
sleep, and will be restarted after waking up.
Signed-off-by: Howard Chung <howardchung@google.com>
Reviewed-by: Alain Michaud <alainm@chromium.org>
Reviewed-by: Manish Mandlik <mmandlik@chromium.org>
Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---
(no changes since v5)
Changes in v5:
- Remove the change in hci_req_config_le_suspend_scan
net/bluetooth/hci_request.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index ba3016cc0b573..db44680fbe9c9 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1281,8 +1281,10 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1, &page_scan);
/* Disable LE passive scan if enabled */
- if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
+ if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
+ cancel_interleave_scan(hdev);
hci_req_add_le_scan_disable(&req, false);
+ }
/* Mark task needing completion */
set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v6 3/4] Bluetooth: Handle active scan case
2020-09-28 7:41 [PATCH v6 1/4] Bluetooth: Interleave with allowlist scan Howard Chung
2020-09-28 7:41 ` [PATCH v6 2/4] Bluetooth: Handle system suspend resume case Howard Chung
@ 2020-09-28 7:41 ` Howard Chung
2020-09-28 7:41 ` [PATCH v6 4/4] Bluetooth: Add toggle to switch off interleave scan Howard Chung
2 siblings, 0 replies; 6+ messages in thread
From: Howard Chung @ 2020-09-28 7:41 UTC (permalink / raw)
To: linux-bluetooth, marcel, luiz.dentz
Cc: alainm, mcchou, mmandlik, Howard Chung, Manish Mandlik,
David S. Miller, Jakub Kicinski, Johan Hedberg, linux-kernel,
netdev
This patch adds code to handle the active scan during interleave
scan. The interleave scan will be canceled when users start active scan,
and it will be restarted after active scan stopped.
Signed-off-by: Howard Chung <howardchung@google.com>
Reviewed-by: Alain Michaud <alainm@chromium.org>
Reviewed-by: Manish Mandlik <mmandlik@chromium.org>
---
(no changes since v1)
net/bluetooth/hci_request.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index db44680fbe9c9..4048c82d4257f 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -3083,8 +3083,10 @@ static int active_scan(struct hci_request *req, unsigned long opt)
* running. Thus, we should temporarily stop it in order to set the
* discovery scanning parameters.
*/
- if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
+ if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
hci_req_add_le_scan_disable(req, false);
+ cancel_interleave_scan(hdev);
+ }
/* All active scans will be done with either a resolvable private
* address (when privacy feature has been enabled) or non-resolvable
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v6 4/4] Bluetooth: Add toggle to switch off interleave scan
2020-09-28 7:41 [PATCH v6 1/4] Bluetooth: Interleave with allowlist scan Howard Chung
2020-09-28 7:41 ` [PATCH v6 2/4] Bluetooth: Handle system suspend resume case Howard Chung
2020-09-28 7:41 ` [PATCH v6 3/4] Bluetooth: Handle active scan case Howard Chung
@ 2020-09-28 7:41 ` Howard Chung
2020-09-28 11:40 ` kernel test robot
2020-09-28 19:45 ` Jakub Kicinski
2 siblings, 2 replies; 6+ messages in thread
From: Howard Chung @ 2020-09-28 7:41 UTC (permalink / raw)
To: linux-bluetooth, marcel, luiz.dentz
Cc: alainm, mcchou, mmandlik, Howard Chung, David S. Miller,
Jakub Kicinski, Johan Hedberg, linux-kernel, netdev
This patch add a configurable parameter to switch off the interleave
scan feature.
Signed-off-by: Howard Chung <howardchung@google.com>
Reviewed-by: Alain Michaud <alainm@chromium.org>
---
Changes in v6:
- Change the type of enable_advmon_interleave_scan to u8
Changes in v4:
- Set EnableAdvMonInterleaveScan default to Disable
- Fix 80 chars limit in mgmt_config.c
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_core.c | 1 +
net/bluetooth/hci_request.c | 3 ++-
net/bluetooth/mgmt_config.c | 39 +++++++++++++++++++++++---------
4 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index cfede18709d8f..63c6d656564a1 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -363,6 +363,7 @@ struct hci_dev {
__u32 clock;
__u16 advmon_allowlist_duration;
__u16 advmon_no_filter_duration;
+ __u8 enable_advmon_interleave_scan;
__u16 devid_source;
__u16 devid_vendor;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 6c8850149265a..c37b2d5395abc 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3595,6 +3595,7 @@ struct hci_dev *hci_alloc_dev(void)
/* The default values will be chosen in the future */
hdev->advmon_allowlist_duration = 300;
hdev->advmon_no_filter_duration = 500;
+ hdev->enable_advmon_interleave_scan = 0x00; /* Default to disable */
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 4048c82d4257f..23381f263678b 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1057,7 +1057,8 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
&own_addr_type))
return;
- if (__hci_update_interleaved_scan(hdev))
+ if (hdev->enable_advmon_interleave_scan &&
+ __hci_update_interleaved_scan(hdev))
return;
/* Adding or removing entries from the white list must
diff --git a/net/bluetooth/mgmt_config.c b/net/bluetooth/mgmt_config.c
index 2d3ad288c78ac..f7906c5d7f01a 100644
--- a/net/bluetooth/mgmt_config.c
+++ b/net/bluetooth/mgmt_config.c
@@ -17,6 +17,12 @@
{ cpu_to_le16(hdev->_param_name_) } \
}
+#define HDEV_PARAM_U8(_param_code_, _param_name_) \
+{ \
+ { (_param_code_), sizeof(__u8) }, \
+ { hdev->_param_name_ } \
+}
+
#define HDEV_PARAM_U16_JIFFIES_TO_MSECS(_param_code_, _param_name_) \
{ \
{ cpu_to_le16(_param_code_), sizeof(__u16) }, \
@@ -30,11 +36,12 @@ int read_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
struct mgmt_tlv entry;
union {
/* This is a simplification for now since all values
- * are 16 bits. In the future, this code may need
+ * are fixed bits. In the future, this code may need
* refactoring to account for variable length values
* and properly calculate the required buffer size.
*/
- __le16 value;
+ __le16 value_le16;
+ __u8 value_u8;
};
} __packed params[] = {
/* Please see mgmt-api.txt for documentation of these values */
@@ -69,6 +76,7 @@ int read_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
def_le_autoconnect_timeout),
HDEV_PARAM_U16(0x001d, advmon_allowlist_duration),
HDEV_PARAM_U16(0x001e, advmon_no_filter_duration),
+ HDEV_PARAM_U8(0x001f, enable_advmon_interleave_scan),
};
struct mgmt_rp_read_def_system_config *rp = (void *)params;
@@ -81,7 +89,7 @@ int read_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
#define TO_TLV(x) ((struct mgmt_tlv *)(x))
#define TLV_GET_LE16(tlv) le16_to_cpu(*((__le16 *)(TO_TLV(tlv)->value)))
-
+#define TLV_GET_U8(tlv) (*((__u8 *)(TO_TLV(tlv)->value)))
int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
u16 data_len)
{
@@ -100,6 +108,7 @@ int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
const u16 exp_len = sizeof(struct mgmt_tlv) +
len;
const u16 type = le16_to_cpu(TO_TLV(buffer)->type);
+ u8 exp_data_len;
if (buffer_left < exp_len) {
bt_dev_warn(hdev, "invalid len left %d, exp >= %d",
@@ -142,20 +151,25 @@ int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
case 0x001b:
case 0x001d:
case 0x001e:
- if (len != sizeof(u16)) {
- bt_dev_warn(hdev, "invalid length %d, exp %zu for type %d",
- len, sizeof(u16), type);
-
- return mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_SET_DEF_SYSTEM_CONFIG,
- MGMT_STATUS_INVALID_PARAMS);
- }
+ exp_data_len = sizeof(u16);
+ break;
+ case 0x001f:
+ exp_data_len = sizeof(u8);
break;
default:
bt_dev_warn(hdev, "unsupported parameter %u", type);
break;
}
+ if (len != exp_data_len) {
+ bt_dev_warn(hdev, "invalid length %d, exp %zu for type %d",
+ len, exp_data_len, type);
+
+ return mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_DEF_SYSTEM_CONFIG,
+ MGMT_STATUS_INVALID_PARAMS);
+ }
+
buffer_left -= exp_len;
buffer += exp_len;
}
@@ -261,6 +275,9 @@ int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
case 0x0001e:
hdev->advmon_no_filter_duration = TLV_GET_LE16(buffer);
break;
+ case 0x0001f:
+ hdev->enable_advmon_interleave_scan = TLV_GET_U8(buffer);
+ break;
default:
bt_dev_warn(hdev, "unsupported parameter %u", type);
break;
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v6 4/4] Bluetooth: Add toggle to switch off interleave scan
2020-09-28 7:41 ` [PATCH v6 4/4] Bluetooth: Add toggle to switch off interleave scan Howard Chung
@ 2020-09-28 11:40 ` kernel test robot
2020-09-28 19:45 ` Jakub Kicinski
1 sibling, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-09-28 11:40 UTC (permalink / raw)
To: Howard Chung, linux-bluetooth, marcel, luiz.dentz
Cc: kbuild-all, clang-built-linux, alainm, mcchou, mmandlik,
Howard Chung, Jakub Kicinski, Johan Hedberg, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 9608 bytes --]
Hi Howard,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on net-next/master net/master v5.9-rc7 next-20200925]
[cannot apply to sparc-next/master]
[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/Howard-Chung/Bluetooth-Interleave-with-allowlist-scan/20200928-154335
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: x86_64-randconfig-r011-20200928 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 6d374cf78c8a80a0bbfc7ce9bc80b3f183f44c80)
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
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/99ac4165fefc0a5c5afc88f7f0527fe45333098d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Howard-Chung/Bluetooth-Interleave-with-allowlist-scan/20200928-154335
git checkout 99ac4165fefc0a5c5afc88f7f0527fe45333098d
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/bluetooth/mgmt_config.c:166:14: warning: format specifies type 'size_t' (aka 'unsigned long') but the argument has type 'u8' (aka 'unsigned char') [-Wformat]
len, exp_data_len, type);
^~~~~~~~~~~~
include/net/bluetooth/bluetooth.h:186:38: note: expanded from macro 'bt_dev_warn'
BT_WARN("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/net/bluetooth/bluetooth.h:174:47: note: expanded from macro 'BT_WARN'
#define BT_WARN(fmt, ...) bt_warn(fmt "\n", ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
>> net/bluetooth/mgmt_config.c:159:3: warning: variable 'exp_data_len' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
default:
^~~~~~~
net/bluetooth/mgmt_config.c:164:14: note: uninitialized use occurs here
if (len != exp_data_len) {
^~~~~~~~~~~~
net/bluetooth/mgmt_config.c:111:18: note: initialize the variable 'exp_data_len' to silence this warning
u8 exp_data_len;
^
= '\0'
2 warnings generated.
vim +166 net/bluetooth/mgmt_config.c
89
90 #define TO_TLV(x) ((struct mgmt_tlv *)(x))
91 #define TLV_GET_LE16(tlv) le16_to_cpu(*((__le16 *)(TO_TLV(tlv)->value)))
92 #define TLV_GET_U8(tlv) (*((__u8 *)(TO_TLV(tlv)->value)))
93 int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
94 u16 data_len)
95 {
96 u16 buffer_left = data_len;
97 u8 *buffer = data;
98
99 if (buffer_left < sizeof(struct mgmt_tlv)) {
100 return mgmt_cmd_status(sk, hdev->id,
101 MGMT_OP_SET_DEF_SYSTEM_CONFIG,
102 MGMT_STATUS_INVALID_PARAMS);
103 }
104
105 /* First pass to validate the tlv */
106 while (buffer_left >= sizeof(struct mgmt_tlv)) {
107 const u8 len = TO_TLV(buffer)->length;
108 const u16 exp_len = sizeof(struct mgmt_tlv) +
109 len;
110 const u16 type = le16_to_cpu(TO_TLV(buffer)->type);
111 u8 exp_data_len;
112
113 if (buffer_left < exp_len) {
114 bt_dev_warn(hdev, "invalid len left %d, exp >= %d",
115 buffer_left, exp_len);
116
117 return mgmt_cmd_status(sk, hdev->id,
118 MGMT_OP_SET_DEF_SYSTEM_CONFIG,
119 MGMT_STATUS_INVALID_PARAMS);
120 }
121
122 /* Please see mgmt-api.txt for documentation of these values */
123 switch (type) {
124 case 0x0000:
125 case 0x0001:
126 case 0x0002:
127 case 0x0003:
128 case 0x0004:
129 case 0x0005:
130 case 0x0006:
131 case 0x0007:
132 case 0x0008:
133 case 0x0009:
134 case 0x000a:
135 case 0x000b:
136 case 0x000c:
137 case 0x000d:
138 case 0x000e:
139 case 0x000f:
140 case 0x0010:
141 case 0x0011:
142 case 0x0012:
143 case 0x0013:
144 case 0x0014:
145 case 0x0015:
146 case 0x0016:
147 case 0x0017:
148 case 0x0018:
149 case 0x0019:
150 case 0x001a:
151 case 0x001b:
152 case 0x001d:
153 case 0x001e:
154 exp_data_len = sizeof(u16);
155 break;
156 case 0x001f:
157 exp_data_len = sizeof(u8);
158 break;
> 159 default:
160 bt_dev_warn(hdev, "unsupported parameter %u", type);
161 break;
162 }
163
164 if (len != exp_data_len) {
165 bt_dev_warn(hdev, "invalid length %d, exp %zu for type %d",
> 166 len, exp_data_len, type);
167
168 return mgmt_cmd_status(sk, hdev->id,
169 MGMT_OP_SET_DEF_SYSTEM_CONFIG,
170 MGMT_STATUS_INVALID_PARAMS);
171 }
172
173 buffer_left -= exp_len;
174 buffer += exp_len;
175 }
176
177 buffer_left = data_len;
178 buffer = data;
179 while (buffer_left >= sizeof(struct mgmt_tlv)) {
180 const u8 len = TO_TLV(buffer)->length;
181 const u16 exp_len = sizeof(struct mgmt_tlv) +
182 len;
183 const u16 type = le16_to_cpu(TO_TLV(buffer)->type);
184
185 switch (type) {
186 case 0x0000:
187 hdev->def_page_scan_type = TLV_GET_LE16(buffer);
188 break;
189 case 0x0001:
190 hdev->def_page_scan_int = TLV_GET_LE16(buffer);
191 break;
192 case 0x0002:
193 hdev->def_page_scan_window = TLV_GET_LE16(buffer);
194 break;
195 case 0x0003:
196 hdev->def_inq_scan_type = TLV_GET_LE16(buffer);
197 break;
198 case 0x0004:
199 hdev->def_inq_scan_int = TLV_GET_LE16(buffer);
200 break;
201 case 0x0005:
202 hdev->def_inq_scan_window = TLV_GET_LE16(buffer);
203 break;
204 case 0x0006:
205 hdev->def_br_lsto = TLV_GET_LE16(buffer);
206 break;
207 case 0x0007:
208 hdev->def_page_timeout = TLV_GET_LE16(buffer);
209 break;
210 case 0x0008:
211 hdev->sniff_min_interval = TLV_GET_LE16(buffer);
212 break;
213 case 0x0009:
214 hdev->sniff_max_interval = TLV_GET_LE16(buffer);
215 break;
216 case 0x000a:
217 hdev->le_adv_min_interval = TLV_GET_LE16(buffer);
218 break;
219 case 0x000b:
220 hdev->le_adv_max_interval = TLV_GET_LE16(buffer);
221 break;
222 case 0x000c:
223 hdev->def_multi_adv_rotation_duration =
224 TLV_GET_LE16(buffer);
225 break;
226 case 0x000d:
227 hdev->le_scan_interval = TLV_GET_LE16(buffer);
228 break;
229 case 0x000e:
230 hdev->le_scan_window = TLV_GET_LE16(buffer);
231 break;
232 case 0x000f:
233 hdev->le_scan_int_suspend = TLV_GET_LE16(buffer);
234 break;
235 case 0x0010:
236 hdev->le_scan_window_suspend = TLV_GET_LE16(buffer);
237 break;
238 case 0x0011:
239 hdev->le_scan_int_discovery = TLV_GET_LE16(buffer);
240 break;
241 case 0x00012:
242 hdev->le_scan_window_discovery = TLV_GET_LE16(buffer);
243 break;
244 case 0x00013:
245 hdev->le_scan_int_adv_monitor = TLV_GET_LE16(buffer);
246 break;
247 case 0x00014:
248 hdev->le_scan_window_adv_monitor = TLV_GET_LE16(buffer);
249 break;
250 case 0x00015:
251 hdev->le_scan_int_connect = TLV_GET_LE16(buffer);
252 break;
253 case 0x00016:
254 hdev->le_scan_window_connect = TLV_GET_LE16(buffer);
255 break;
256 case 0x00017:
257 hdev->le_conn_min_interval = TLV_GET_LE16(buffer);
258 break;
259 case 0x00018:
260 hdev->le_conn_max_interval = TLV_GET_LE16(buffer);
261 break;
262 case 0x00019:
263 hdev->le_conn_latency = TLV_GET_LE16(buffer);
264 break;
265 case 0x0001a:
266 hdev->le_supv_timeout = TLV_GET_LE16(buffer);
267 break;
268 case 0x0001b:
269 hdev->def_le_autoconnect_timeout =
270 msecs_to_jiffies(TLV_GET_LE16(buffer));
271 break;
272 case 0x0001d:
273 hdev->advmon_allowlist_duration = TLV_GET_LE16(buffer);
274 break;
275 case 0x0001e:
276 hdev->advmon_no_filter_duration = TLV_GET_LE16(buffer);
277 break;
278 case 0x0001f:
279 hdev->enable_advmon_interleave_scan = TLV_GET_U8(buffer);
280 break;
281 default:
282 bt_dev_warn(hdev, "unsupported parameter %u", type);
283 break;
284 }
285
286 buffer_left -= exp_len;
287 buffer += exp_len;
288 }
289
290 return mgmt_cmd_complete(sk, hdev->id,
291 MGMT_OP_SET_DEF_SYSTEM_CONFIG, 0, NULL, 0);
292 }
293
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 44468 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6 4/4] Bluetooth: Add toggle to switch off interleave scan
2020-09-28 7:41 ` [PATCH v6 4/4] Bluetooth: Add toggle to switch off interleave scan Howard Chung
2020-09-28 11:40 ` kernel test robot
@ 2020-09-28 19:45 ` Jakub Kicinski
1 sibling, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2020-09-28 19:45 UTC (permalink / raw)
To: Howard Chung
Cc: linux-bluetooth, marcel, luiz.dentz, alainm, mcchou, mmandlik,
David S. Miller, Johan Hedberg, linux-kernel, netdev
On Mon, 28 Sep 2020 15:41:21 +0800 Howard Chung wrote:
> This patch add a configurable parameter to switch off the interleave
> scan feature.
>
> Signed-off-by: Howard Chung <howardchung@google.com>
> Reviewed-by: Alain Michaud <alainm@chromium.org>
This seems to cause new warnings on W=1 C=1 builds:
In file included from ../net/bluetooth/mgmt_config.c:7:
net/bluetooth/mgmt_config.c: In function ‘set_def_system_config’:
include/net/bluetooth/bluetooth.h:186:10: warning: format ‘%zu’ expects argument of type ‘size_t’, but argument 4 has type ‘int’ [-Wformat=]
186 | BT_WARN("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
| ^~~~~~
include/net/bluetooth/bluetooth.h:174:35: note: in definition of macro ‘BT_WARN’
174 | #define BT_WARN(fmt, ...) bt_warn(fmt "\n", ##__VA_ARGS__)
| ^~~
net/bluetooth/mgmt_config.c:165:4: note: in expansion of macro ‘bt_dev_warn’
165 | bt_dev_warn(hdev, "invalid length %d, exp %zu for type %d",
| ^~~~~~~~~~~
net/bluetooth/mgmt_config.c:79:17: warning: incorrect type in initializer (different base types)
net/bluetooth/mgmt_config.c:79:17: expected restricted __le16 [usertype] type
net/bluetooth/mgmt_config.c:79:17: got int
net/bluetooth/mgmt_config.c:79:17: warning: incorrect type in initializer (different base types)
net/bluetooth/mgmt_config.c:79:17: expected restricted __le16 [usertype] value_le16
net/bluetooth/mgmt_config.c:79:17: got unsigned char [usertype]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-28 19:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-28 7:41 [PATCH v6 1/4] Bluetooth: Interleave with allowlist scan Howard Chung
2020-09-28 7:41 ` [PATCH v6 2/4] Bluetooth: Handle system suspend resume case Howard Chung
2020-09-28 7:41 ` [PATCH v6 3/4] Bluetooth: Handle active scan case Howard Chung
2020-09-28 7:41 ` [PATCH v6 4/4] Bluetooth: Add toggle to switch off interleave scan Howard Chung
2020-09-28 11:40 ` kernel test robot
2020-09-28 19:45 ` Jakub Kicinski
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).