2023-11-22 17:56:58

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [RESEND] Bluetooth: hci_core: Remove le_restart_scan work

From: Luiz Augusto von Dentz <[email protected]>

This removes le_restart_scan work and instead just disables controller
duplicate filtering when discovery result_filtering is enabled and
HCI_QUIRK_STRICT_DUPLICATE_FILTER is set.

Link: https://github.com/bluez/bluez/issues/573
Link: https://github.com/bluez/bluez/issues/572
Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 -
net/bluetooth/hci_sync.c | 96 +++-----------------------------
net/bluetooth/mgmt.c | 17 ------
3 files changed, 7 insertions(+), 107 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 201c0809540a..eed6c9f37b12 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -534,7 +534,6 @@ struct hci_dev {
struct work_struct tx_work;

struct delayed_work le_scan_disable;
- struct delayed_work le_scan_restart;

struct sk_buff_head rx_q;
struct sk_buff_head raw_q;
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index d85a7091a116..3563a90ed2ac 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -348,8 +348,6 @@ static void le_scan_disable(struct work_struct *work)
if (!hci_dev_test_flag(hdev, HCI_LE_SCAN))
goto _return;

- cancel_delayed_work(&hdev->le_scan_restart);
-
status = hci_cmd_sync_queue(hdev, scan_disable_sync, NULL, NULL);
if (status) {
bt_dev_err(hdev, "failed to disable LE scan: %d", status);
@@ -397,71 +395,6 @@ static void le_scan_disable(struct work_struct *work)

static int hci_le_set_scan_enable_sync(struct hci_dev *hdev, u8 val,
u8 filter_dup);
-static int hci_le_scan_restart_sync(struct hci_dev *hdev)
-{
- /* If controller is not scanning we are done. */
- if (!hci_dev_test_flag(hdev, HCI_LE_SCAN))
- return 0;
-
- if (hdev->scanning_paused) {
- bt_dev_dbg(hdev, "Scanning is paused for suspend");
- return 0;
- }
-
- hci_le_set_scan_enable_sync(hdev, LE_SCAN_DISABLE, 0x00);
- return hci_le_set_scan_enable_sync(hdev, LE_SCAN_ENABLE,
- LE_SCAN_FILTER_DUP_ENABLE);
-}
-
-static void le_scan_restart(struct work_struct *work)
-{
- struct hci_dev *hdev = container_of(work, struct hci_dev,
- le_scan_restart.work);
- unsigned long timeout, duration, scan_start, now;
- int status;
-
- bt_dev_dbg(hdev, "");
-
- status = hci_le_scan_restart_sync(hdev);
- if (status) {
- bt_dev_err(hdev, "failed to restart LE scan: status %d",
- status);
- return;
- }
-
- hci_dev_lock(hdev);
-
- if (!test_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks) ||
- !hdev->discovery.scan_start)
- goto unlock;
-
- /* When the scan was started, hdev->le_scan_disable has been queued
- * after duration from scan_start. During scan restart this job
- * has been canceled, and we need to queue it again after proper
- * timeout, to make sure that scan does not run indefinitely.
- */
- duration = hdev->discovery.scan_duration;
- scan_start = hdev->discovery.scan_start;
- now = jiffies;
- if (now - scan_start <= duration) {
- int elapsed;
-
- if (now >= scan_start)
- elapsed = now - scan_start;
- else
- elapsed = ULONG_MAX - scan_start + now;
-
- timeout = duration - elapsed;
- } else {
- timeout = 0;
- }
-
- queue_delayed_work(hdev->req_workqueue,
- &hdev->le_scan_disable, timeout);
-
-unlock:
- hci_dev_unlock(hdev);
-}

static int reenable_adv_sync(struct hci_dev *hdev, void *data)
{
@@ -630,7 +563,6 @@ void hci_cmd_sync_init(struct hci_dev *hdev)
INIT_WORK(&hdev->cmd_sync_cancel_work, hci_cmd_sync_cancel_work);
INIT_WORK(&hdev->reenable_adv_work, reenable_adv);
INIT_DELAYED_WORK(&hdev->le_scan_disable, le_scan_disable);
- INIT_DELAYED_WORK(&hdev->le_scan_restart, le_scan_restart);
INIT_DELAYED_WORK(&hdev->adv_instance_expire, adv_timeout_expire);
}

@@ -4960,7 +4892,6 @@ int hci_dev_close_sync(struct hci_dev *hdev)
cancel_delayed_work(&hdev->power_off);
cancel_delayed_work(&hdev->ncmd_timer);
cancel_delayed_work(&hdev->le_scan_disable);
- cancel_delayed_work(&hdev->le_scan_restart);

hci_request_cancel_all(hdev);

@@ -5178,7 +5109,6 @@ int hci_stop_discovery_sync(struct hci_dev *hdev)

if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
cancel_delayed_work(&hdev->le_scan_disable);
- cancel_delayed_work(&hdev->le_scan_restart);

err = hci_scan_disable_sync(hdev);
if (err)
@@ -5686,19 +5616,18 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
if (err < 0)
own_addr_type = ADDR_LE_DEV_PUBLIC;

- if (hci_is_adv_monitoring(hdev)) {
+ if (hci_is_adv_monitoring(hdev) ||
+ (test_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks) &&
+ hdev->discovery.result_filtering)) {
/* 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. Although in the
- * latter case we don't need to disable duplicate filter, but
- * it is common to have active scanning for a short period of
- * time, the power impact should be neglectable.
+ * If controller does strict duplicate filtering and the
+ * discovery requires result filtering disables controller based
+ * filtering since that can cause reports that would match the
+ * host filter to not be reported.
*/
filter_dup = LE_SCAN_FILTER_DUP_DISABLE;
}
@@ -5778,17 +5707,6 @@ int hci_start_discovery_sync(struct hci_dev *hdev)

bt_dev_dbg(hdev, "timeout %u ms", jiffies_to_msecs(timeout));

- /* When service discovery is used and the controller has a
- * strict duplicate filter, it is important to remember the
- * start and duration of the scan. This is required for
- * restarting scanning during the discovery phase.
- */
- if (test_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks) &&
- hdev->discovery.result_filtering) {
- hdev->discovery.scan_start = jiffies;
- hdev->discovery.scan_duration = timeout;
- }
-
queue_delayed_work(hdev->req_workqueue, &hdev->le_scan_disable,
timeout);
return 0;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ba2e00646e8e..da79a2369dd7 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -10134,21 +10134,6 @@ static bool eir_has_uuids(u8 *eir, u16 eir_len, u16 uuid_count, u8 (*uuids)[16])
return false;
}

-static void restart_le_scan(struct hci_dev *hdev)
-{
- /* If controller is not scanning we are done. */
- if (!hci_dev_test_flag(hdev, HCI_LE_SCAN))
- return;
-
- if (time_after(jiffies + DISCOV_LE_RESTART_DELAY,
- hdev->discovery.scan_start +
- hdev->discovery.scan_duration))
- return;
-
- queue_delayed_work(hdev->req_workqueue, &hdev->le_scan_restart,
- DISCOV_LE_RESTART_DELAY);
-}
-
static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
{
@@ -10183,8 +10168,6 @@ static bool is_filter_match(struct hci_dev *hdev, s8 rssi, u8 *eir,
* scanning to ensure updated result with updated RSSI values.
*/
if (test_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks)) {
- restart_le_scan(hdev);
-
/* Validate RSSI value against the RSSI threshold once more. */
if (hdev->discovery.rssi != HCI_RSSI_INVALID &&
rssi < hdev->discovery.rssi)
--
2.42.0



2023-11-22 18:45:27

by bluez.test.bot

[permalink] [raw]
Subject: RE: [RESEND] Bluetooth: hci_core: Remove le_restart_scan work

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=803389

---Test result---

Test Summary:
CheckPatch PASS 1.03 seconds
GitLint PASS 0.28 seconds
SubjectPrefix PASS 0.06 seconds
BuildKernel PASS 28.27 seconds
CheckAllWarning PASS 31.23 seconds
CheckSparse PASS 36.70 seconds
CheckSmatch PASS 101.34 seconds
BuildKernel32 PASS 27.83 seconds
TestRunnerSetup PASS 432.11 seconds
TestRunner_l2cap-tester PASS 23.97 seconds
TestRunner_iso-tester PASS 40.21 seconds
TestRunner_bnep-tester PASS 7.22 seconds
TestRunner_mgmt-tester PASS 166.49 seconds
TestRunner_rfcomm-tester PASS 13.10 seconds
TestRunner_sco-tester PASS 14.69 seconds
TestRunner_ioctl-tester PASS 12.21 seconds
TestRunner_mesh-tester PASS 8.93 seconds
TestRunner_smp-tester PASS 9.87 seconds
TestRunner_userchan-tester PASS 7.40 seconds
IncrementalBuild PASS 25.55 seconds



---
Regards,
Linux Bluetooth

2023-11-27 15:20:39

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [RESEND] Bluetooth: hci_core: Remove le_restart_scan work

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Wed, 22 Nov 2023 12:56:36 -0500 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This removes le_restart_scan work and instead just disables controller
> duplicate filtering when discovery result_filtering is enabled and
> HCI_QUIRK_STRICT_DUPLICATE_FILTER is set.
>
> Link: https://github.com/bluez/bluez/issues/573
> Link: https://github.com/bluez/bluez/issues/572
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
>
> [...]

Here is the summary with links:
- [RESEND] Bluetooth: hci_core: Remove le_restart_scan work
https://git.kernel.org/bluetooth/bluetooth-next/c/a4230d16e0b6

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html