diff mbox series

[v1] Bluetooth: disable filter dup when scan for adv monitor

Message ID 20210519102745.v1.1.I69e82377dd94ad7cba0cde75bcac2dce62fbc542@changeid (mailing list archive)
State New, archived
Headers show
Series [v1] Bluetooth: disable filter dup when scan for adv monitor | expand

Commit Message

Howard Chung May 19, 2021, 2:28 a.m. UTC 
From: Yun-Hao Chung <howardchung@chromium.org>

Disable duplicates filter when scanning for advertisement monitor for
the following reasons. The scanning includes active scan and passive
scan.

For HW pattern filtering (ex. MSFT), some controllers ignore
RSSI_Sampling_Period when the duplicates filter is enabled.

For SW pattern filtering, when we're not doing interleaved scanning, it
is necessary to disable duplicates filter, otherwise hosts can only
receive one advertisement and it's impossible to know if a peer is still
in range.

Reviewed-by: Archie Pusaka <apusaka@chromium.org>

Signed-off-by: Yun-Hao Chung <howardchung@chromium.org>
---

 net/bluetooth/hci_request.c | 42 ++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

Comments

bluez.test.bot@gmail.com May 19, 2021, 3:29 a.m. UTC | #1 
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=484667

---Test result---

Test Summary:
CheckPatch                    PASS      0.63 seconds
GitLint                       PASS      0.11 seconds
BuildKernel                   PASS      555.29 seconds
TestRunner: Setup             PASS      367.77 seconds
TestRunner: l2cap-tester      PASS      2.65 seconds
TestRunner: bnep-tester       PASS      2.00 seconds
TestRunner: mgmt-tester       PASS      27.56 seconds
TestRunner: rfcomm-tester     PASS      2.18 seconds
TestRunner: sco-tester        PASS      2.08 seconds
TestRunner: smp-tester        PASS      2.17 seconds
TestRunner: userchan-tester   PASS      1.94 seconds

Details
##############################
Test: CheckPatch - PASS - 0.63 seconds
Run checkpatch.pl script with rule in .checkpatch.conf


##############################
Test: GitLint - PASS - 0.11 seconds
Run gitlint with rule in .gitlint


##############################
Test: BuildKernel - PASS - 555.29 seconds
Build Kernel with minimal configuration supports Bluetooth


##############################
Test: TestRunner: Setup - PASS - 367.77 seconds
Setup environment for running Test Runner


##############################
Test: TestRunner: l2cap-tester - PASS - 2.65 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: bnep-tester - PASS - 2.00 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: mgmt-tester - PASS - 27.56 seconds
Run test-runner with mgmt-tester
Total: 416, Passed: 403 (96.9%), Failed: 0, Not Run: 13

##############################
Test: TestRunner: rfcomm-tester - PASS - 2.18 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: sco-tester - PASS - 2.08 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: smp-tester - PASS - 2.17 seconds
Run test-runner with smp-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: userchan-tester - PASS - 1.94 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth
Marcel Holtmann May 19, 2021, 8:47 p.m. UTC | #2 
Hi Howard,

> Disable duplicates filter when scanning for advertisement monitor for
> the following reasons. The scanning includes active scan and passive
> scan.
> 
> For HW pattern filtering (ex. MSFT), some controllers ignore
> RSSI_Sampling_Period when the duplicates filter is enabled.
> 
> For SW pattern filtering, when we're not doing interleaved scanning, it
> is necessary to disable duplicates filter, otherwise hosts can only
> receive one advertisement and it's impossible to know if a peer is still
> in range.

can we be a bit more specific on which controller does what. I am not inclined to always disable duplicate filtering unless your controller doesn’t do what you want it to do.

I also disagree with the last statement. If the device moved out of range (or comes back for that matter) you should get a HCI_VS_MSFT_LE_Monitor_Device_Event event that tells you if a device is in range or not.

Device leaving:

> HCI Event: LE Meta Event (0x3e) plen 43
      LE Advertising Report (0x02)
        Num reports: 1
        Event type: Non connectable undirected - ADV_NONCONN_IND (0x03)
        Address type: Random (0x01)
        Address: 01:9A:1F:C0:30:15 (Non-Resolvable)
        Data length: 31
        Flags: 0x1a
          LE General Discoverable Mode
          Simultaneous LE and BR/EDR (Controller)
          Simultaneous LE and BR/EDR (Host)
        16-bit Service UUIDs (complete): 1 entry
          Apple, Inc. (0xfd6f)
        Service Data (UUID 0xfd6f): f47698ff9243617d917ac521b5fcfd436afdb285
        RSSI: -86 dBm (0xaa)
> HCI Event: Vendor (0xff) plen 18
        23 79 54 33 77 88 97 68 02 01 15 30 c0 1f 9a 01  #yT3w..h...0....
        00 00                                            ..              

Device coming back:

> HCI Event: Vendor (0xff) plen 18
        23 79 54 33 77 88 97 68 02 01 95 b9 0b 32 22 2a  #yT3w..h.....2"*
        00 01                                            ..              
> HCI Event: LE Meta Event (0x3e) plen 43
      LE Advertising Report (0x02)
        Num reports: 1
        Event type: Non connectable undirected - ADV_NONCONN_IND (0x03)
        Address type: Random (0x01)
        Address: 2A:22:32:0B:B9:95 (Non-Resolvable)
        Data length: 31
        Flags: 0x1a
          LE General Discoverable Mode
          Simultaneous LE and BR/EDR (Controller)
          Simultaneous LE and BR/EDR (Host)
        16-bit Service UUIDs (complete): 1 entry
          Apple, Inc. (0xfd6f)
        Service Data (UUID 0xfd6f): 0b861791a0fb7adcf8b45f951f7d4b7c7fc8e3fd
        RSSI: -27 dBm (0xe5)

Regards

Marcel
Howard Chung May 20, 2021, 3:23 a.m. UTC | #3 
Hi Marcel,
Thanks for the comments.

On Thu, May 20, 2021 at 4:47 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Howard,
>
> > Disable duplicates filter when scanning for advertisement monitor for
> > the following reasons. The scanning includes active scan and passive
> > scan.
> >
> > For HW pattern filtering (ex. MSFT), some controllers ignore
> > RSSI_Sampling_Period when the duplicates filter is enabled.
> >
> > For SW pattern filtering, when we're not doing interleaved scanning, it
> > is necessary to disable duplicates filter, otherwise hosts can only
> > receive one advertisement and it's impossible to know if a peer is still
> > in range.
>
> can we be a bit more specific on which controller does what. I am not inclined to always disable duplicate filtering unless your controller doesn’t do what you want it to do.

Will update the commit message and submit again.

>
> I also disagree with the last statement. If the device moved out of range (or comes back for that matter) you should get a HCI_VS_MSFT_LE_Monitor_Device_Event event that tells you if a device is in range or not.

The last statement is about software filtering, which is used when
MSFT is not supported. Software filtering in the kernel is basically
doing an LE passive scan. When the duplicate filter is enabled, some
controllers consider packets with the same address but different RSSIs
as duplicate thus not reporting to the host, which makes userspace not
able to tell if a peer is in range or not.
>
> Device leaving:
>
> > HCI Event: LE Meta Event (0x3e) plen 43
>       LE Advertising Report (0x02)
>         Num reports: 1
>         Event type: Non connectable undirected - ADV_NONCONN_IND (0x03)
>         Address type: Random (0x01)
>         Address: 01:9A:1F:C0:30:15 (Non-Resolvable)
>         Data length: 31
>         Flags: 0x1a
>           LE General Discoverable Mode
>           Simultaneous LE and BR/EDR (Controller)
>           Simultaneous LE and BR/EDR (Host)
>         16-bit Service UUIDs (complete): 1 entry
>           Apple, Inc. (0xfd6f)
>         Service Data (UUID 0xfd6f): f47698ff9243617d917ac521b5fcfd436afdb285
>         RSSI: -86 dBm (0xaa)
> > HCI Event: Vendor (0xff) plen 18
>         23 79 54 33 77 88 97 68 02 01 15 30 c0 1f 9a 01  #yT3w..h...0....
>         00 00                                            ..
>
> Device coming back:
>
> > HCI Event: Vendor (0xff) plen 18
>         23 79 54 33 77 88 97 68 02 01 95 b9 0b 32 22 2a  #yT3w..h.....2"*
>         00 01                                            ..
> > HCI Event: LE Meta Event (0x3e) plen 43
>       LE Advertising Report (0x02)
>         Num reports: 1
>         Event type: Non connectable undirected - ADV_NONCONN_IND (0x03)
>         Address type: Random (0x01)
>         Address: 2A:22:32:0B:B9:95 (Non-Resolvable)
>         Data length: 31
>         Flags: 0x1a
>           LE General Discoverable Mode
>           Simultaneous LE and BR/EDR (Controller)
>           Simultaneous LE and BR/EDR (Host)
>         16-bit Service UUIDs (complete): 1 entry
>           Apple, Inc. (0xfd6f)
>         Service Data (UUID 0xfd6f): 0b861791a0fb7adcf8b45f951f7d4b7c7fc8e3fd
>         RSSI: -27 dBm (0xe5)
>
> Regards
>
> Marcel
>
diff mbox series

Patch

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index fa9125b782f85..54be4f112ef55 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -932,7 +932,7 @@  static bool scan_use_rpa(struct hci_dev *hdev)
 
 static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
 			       u16 window, u8 own_addr_type, u8 filter_policy,
-			       bool addr_resolv)
+			       bool filter_dup, bool addr_resolv)
 {
 	struct hci_dev *hdev = req->hdev;
 
@@ -997,7 +997,7 @@  static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
 
 		memset(&ext_enable_cp, 0, sizeof(ext_enable_cp));
 		ext_enable_cp.enable = LE_SCAN_ENABLE;
-		ext_enable_cp.filter_dup = LE_SCAN_FILTER_DUP_ENABLE;
+		ext_enable_cp.filter_dup = filter_dup;
 
 		hci_req_add(req, HCI_OP_LE_SET_EXT_SCAN_ENABLE,
 			    sizeof(ext_enable_cp), &ext_enable_cp);
@@ -1016,7 +1016,7 @@  static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
 
 		memset(&enable_cp, 0, sizeof(enable_cp));
 		enable_cp.enable = LE_SCAN_ENABLE;
-		enable_cp.filter_dup = LE_SCAN_FILTER_DUP_ENABLE;
+		enable_cp.filter_dup = filter_dup;
 		hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(enable_cp),
 			    &enable_cp);
 	}
@@ -1053,6 +1053,8 @@  void hci_req_add_le_passive_scan(struct hci_request *req)
 	u8 own_addr_type;
 	u8 filter_policy;
 	u16 window, interval;
+	/* Default is to enable duplicates filter */
+	u8 filter_dup = LE_SCAN_FILTER_DUP_ENABLE;
 	/* Background scanning should run with address resolution */
 	bool addr_resolv = true;
 
@@ -1106,6 +1108,19 @@  void hci_req_add_le_passive_scan(struct hci_request *req)
 	} else if (hci_is_adv_monitoring(hdev)) {
 		window = hdev->le_scan_window_adv_monitor;
 		interval = hdev->le_scan_int_adv_monitor;
+
+		/* Disable duplicates filter when scanning for advertisement
+		 * monitor for the following reasons.
+		 *
+		 * For HW pattern filtering (ex. MSFT), some controllers ignore
+		 * RSSI_Sampling_Period when the duplicates filter is enabled.
+		 *
+		 * For SW pattern filtering, when we're not doing interleaved
+		 * scanning, it is necessary to disable duplicates filter,
+		 * otherwise hosts can only receive one advertisement and it's
+		 * impossible to know if a peer is still in range.
+		 */
+		filter_dup = LE_SCAN_FILTER_DUP_DISABLE;
 	} else {
 		window = hdev->le_scan_window;
 		interval = hdev->le_scan_interval;
@@ -1113,7 +1128,8 @@  void hci_req_add_le_passive_scan(struct hci_request *req)
 
 	bt_dev_dbg(hdev, "LE passive scan with whitelist = %d", filter_policy);
 	hci_req_start_scan(req, LE_SCAN_PASSIVE, interval, window,
-			   own_addr_type, filter_policy, addr_resolv);
+			   own_addr_type, filter_policy, filter_dup,
+			   addr_resolv);
 }
 
 static bool adv_instance_is_scannable(struct hci_dev *hdev, u8 instance)
@@ -3135,6 +3151,8 @@  static int active_scan(struct hci_request *req, unsigned long opt)
 	u8 own_addr_type;
 	/* White list is not used for discovery */
 	u8 filter_policy = 0x00;
+	/* Default is to enable duplicates filter */
+	u8 filter_dup = LE_SCAN_FILTER_DUP_ENABLE;
 	/* Discovery doesn't require controller address resolution */
 	bool addr_resolv = false;
 	int err;
@@ -3159,9 +3177,23 @@  static int active_scan(struct hci_request *req, unsigned long opt)
 	if (err < 0)
 		own_addr_type = ADDR_LE_DEV_PUBLIC;
 
+	if (hci_is_adv_monitoring(hdev)) {
+		/* Duplicate filter should be disabled when some advertisement
+		 * monitor is activated, otherwise AdvMon can only receive one
+		 * advertisement for one peer(*) during active scanning, and
+		 * might report loss to these peers.
+		 *
+		 * Note that different controllers have different meanings of
+		 * |duplicate|. Some of them consider packets with the same
+		 * address as duplicate, and others consider packets with the
+		 * same address and the same RSSI as duplicate.
+		 */
+		filter_dup = LE_SCAN_FILTER_DUP_DISABLE;
+	}
+
 	hci_req_start_scan(req, LE_SCAN_ACTIVE, interval,
 			   hdev->le_scan_window_discovery, own_addr_type,
-			   filter_policy, addr_resolv);
+			   filter_policy, filter_dup, addr_resolv);
 	return 0;
 }