This patch add a configurable parameter to switch off the interleave
scan feature.
Signed-off-by: Howard Chung <[email protected]>
Reviewed-by: Alain Michaud <[email protected]>
---
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
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 <[email protected]>
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/[email protected]
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 <[email protected]>
> Reviewed-by: Alain Michaud <[email protected]>
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]