2022-06-16 09:37:05

by Ahmad Fatoum

[permalink] [raw]
Subject: [PATCH] Bluetooth: hci_sync: complete LE connection on any event

Commit 6cd29ec6ae5e ("Bluetooth: hci_sync: Wait for proper events when
connecting LE") changed HCI core to wait for specific events before
posting completion for a new LE connection. This commit introduced
regressions partially fixed in commits a56a1138cbd8
("Bluetooth: hci_sync: Fix not using conn_timeout") and
and c9f73a2178c1 ("Bluetooth: hci_conn: Fix hci_connect_le_sync").

Yet, a regression still remains where devices that worked previously
now timeout[1][2]. Restore working order by reverting the commit in
question until this issue can be properly resolved.

[1]: https://lore.kernel.org/linux-bluetooth/[email protected]/
[2]: https://github.com/bluez/bluez/issues/340

Fixes: 6cd29ec6ae5e ("Bluetooth: hci_sync: Wait for proper events when connecting LE")
Signed-off-by: Ahmad Fatoum <[email protected]>
---
Event callbacks like hci_le_meta_evt() use hci_skb_event(hdev->sent_cmd)
for matching. I suspect the timeout is due to intermittent frames,
e.g. because of remote unpairing, replacing the sent_cmd and thus
breaking renewed pairing as the frames couldn't be matched. This is too
complex for me to fix and we have been carrying this fix for a month now,
so I think it's best we revert this upstream for now.

#regzb Link: https://lore.kernel.org/linux-bluetooth/[email protected]/
Cc: [email protected]
---
net/bluetooth/hci_sync.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 286d6767f017..4cecf15922d4 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5272,7 +5272,7 @@ static int hci_le_ext_create_conn_sync(struct hci_dev *hdev,

return __hci_cmd_sync_status_sk(hdev, HCI_OP_LE_EXT_CREATE_CONN,
plen, data,
- HCI_EV_LE_ENHANCED_CONN_COMPLETE,
+ 0,
conn->conn_timeout, NULL);
}

@@ -5366,9 +5366,7 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn)
*/
err = __hci_cmd_sync_status_sk(hdev, HCI_OP_LE_CREATE_CONN,
sizeof(cp), &cp,
- use_enhanced_conn_complete(hdev) ?
- HCI_EV_LE_ENHANCED_CONN_COMPLETE :
- HCI_EV_LE_CONN_COMPLETE,
+ 0,
conn->conn_timeout, NULL);

done:
--
2.30.2


2022-06-16 10:10:32

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: hci_sync: complete LE connection on any event

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

---Test result---

Test Summary:
CheckPatch FAIL 1.24 seconds
GitLint FAIL 0.68 seconds
SubjectPrefix PASS 0.57 seconds
BuildKernel PASS 30.24 seconds
BuildKernel32 PASS 26.88 seconds
Incremental Build with patchesPASS 37.54 seconds
TestRunner: Setup PASS 461.76 seconds
TestRunner: l2cap-tester PASS 12.67 seconds
TestRunner: bnep-tester PASS 5.63 seconds
TestRunner: mgmt-tester PASS 96.61 seconds
TestRunner: rfcomm-tester PASS 9.16 seconds
TestRunner: sco-tester PASS 8.80 seconds
TestRunner: smp-tester PASS 8.85 seconds
TestRunner: userchan-tester PASS 5.89 seconds

Details
##############################
Test: CheckPatch - FAIL - 1.24 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
Bluetooth: hci_sync: complete LE connection on any event\WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#77:
[1]: https://lore.kernel.org/linux-bluetooth/[email protected]/

total: 0 errors, 1 warnings, 0 checks, 18 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12883653.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL - 0.68 seconds
Run gitlint with rule in .gitlint
Bluetooth: hci_sync: complete LE connection on any event
14: B1 Line exceeds max length (97>80): "[1]: https://lore.kernel.org/linux-bluetooth/[email protected]/"




---
Regards,
Linux Bluetooth

Subject: Re: [PATCH] Bluetooth: hci_sync: complete LE connection on any event

Hi, this is your Linux kernel regression tracker.

On 16.06.22 11:24, Ahmad Fatoum wrote:
> Commit 6cd29ec6ae5e ("Bluetooth: hci_sync: Wait for proper events when
> connecting LE") changed HCI core to wait for specific events before
> posting completion for a new LE connection. This commit introduced
> regressions partially fixed in commits a56a1138cbd8
> ("Bluetooth: hci_sync: Fix not using conn_timeout") and
> and c9f73a2178c1 ("Bluetooth: hci_conn: Fix hci_connect_le_sync").
>
> Yet, a regression still remains where devices that worked previously
> now timeout[1][2]. Restore working order by reverting the commit in
> question until this issue can be properly resolved.
>
> [1]: https://lore.kernel.org/linux-bluetooth/[email protected]/
> [2]: https://github.com/bluez/bluez/issues/340

That way regzbot will miss the tags and not associate the fix with the
regression. You can leave it like that and add these, as explained in
the kernel docs (see for example submitting-patches.rst):

Link:
https://lore.kernel.org/linux-bluetooth/[email protected]/
Link: https://github.com/bluez/bluez/issues/340

Or your remove the two footnotes and do it like this:

Link:
https://lore.kernel.org/linux-bluetooth/[email protected]/
[1]
Link: https://github.com/bluez/bluez/issues/340 [2]

> Fixes: 6cd29ec6ae5e ("Bluetooth: hci_sync: Wait for proper events when connecting LE")
> Signed-off-by: Ahmad Fatoum <[email protected]>
> ---
> Event callbacks like hci_le_meta_evt() use hci_skb_event(hdev->sent_cmd)
> for matching. I suspect the timeout is due to intermittent frames,
> e.g. because of remote unpairing, replacing the sent_cmd and thus
> breaking renewed pairing as the frames couldn't be matched. This is too
> complex for me to fix and we have been carrying this fix for a month now,
> so I think it's best we revert this upstream for now.
>
> #regzb Link: https://lore.kernel.org/linux-bluetooth/[email protected]/

Thx for trying to do the right thing, but that didn't work out, as
"Link:" doesn't need a "#regzb". Maybe regzbot should handle this, not
sure, will keep it in mind.

Anyway, let's make regzbot aware of this thread, as you intended (thx
again):

#regzbot ^backmonitor
https://lore.kernel.org/linux-bluetooth/[email protected]/

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

2022-06-20 10:05:45

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: hci_sync: complete LE connection on any event

Hello Thorsten,

On 19.06.22 13:57, Thorsten Leemhuis wrote:
> Link:
> https://lore.kernel.org/linux-bluetooth/[email protected]/
> [1]
> Link: https://github.com/bluez/bluez/issues/340 [2]

That looks good. Will do it this way in the future.

>> #regzb Link: https://lore.kernel.org/linux-bluetooth/[email protected]/
>
> Thx for trying to do the right thing, but that didn't work out, as
> "Link:" doesn't need a "#regzb". Maybe regzbot should handle this, not
> sure, will keep it in mind.

I copied the #regzb Link: approach off the last snippet here:

https://linux-regtracking.leemhuis.info/post/regzbot-approach/#even-more-problems-in-the-details

Cheers,
Ahmad


--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |