2021-02-17 14:29:14

by Alain Michaud

[permalink] [raw]
Subject: [PATCH v1] Bluetooth: Tolerate valid pre 4.2 conn params

This patch tolerates connection parameters that were valid before
ESR08_v1.0.0 which was also incoporated into the 4.2 core specification.

In particular, this addresses compatibility issues with the Lenovo Yoga
Mouse that sees its connection parameters rejected by the max_latency
computation in hci_conn_check_params, but was perfectly valid at the
time this mouse was qualified.

Before the patch, the mouse was extremely laggy, after the patch, the
mouse worked as expected.

Signed-off-by: Alain Michaud <[email protected]>
Reviewed-by: Miao-chen Chou <[email protected]>
Tested-by: Harry Cutts <[email protected]>

---

include/net/bluetooth/hci_core.h | 30 ++++++++++++++++++++++++++++++
net/bluetooth/l2cap_core.c | 12 ++++++++++++
2 files changed, 42 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ebdd4afe30d2..67b75077c82e 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1646,6 +1646,36 @@ static inline struct smp_irk *hci_get_irk(struct hci_dev *hdev,
return hci_find_irk_by_rpa(hdev, bdaddr);
}

+/* Erratum 5412 which has been fixed in 4.2 changed the validation of
+ * connection parameters. For backwards compatibility reasons, the old
+ * calculation must be tolerated.
+ * For further details :
+ * https://www.bluetooth.org/errata/errata_view.cfm?errata_id=5419
+ */
+static inline int hci_check_conn_params_legacy(u16 min, u16 max, u16 latency,
+ u16 to_multiplier)
+{
+ u16 max_latency;
+
+ if (min > max || min < 6 || max > 3200)
+ return -EINVAL;
+
+ if (to_multiplier < 10 || to_multiplier > 3200)
+ return -EINVAL;
+
+ if (max >= to_multiplier * 8)
+ return -EINVAL;
+
+ max_latency = (to_multiplier * 8 / max) - 1;
+ if (latency > 499 || latency > max_latency)
+ return -EINVAL;
+
+ return 0;
+}
+
+/* Connection Parameter Validation Helper.
+ * See Vol 6, Part B, section 4.5.1.
+ */
static inline int hci_check_conn_params(u16 min, u16 max, u16 latency,
u16 to_multiplier)
{
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 72c2f5226d67..726e78bd85ff 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5553,6 +5553,18 @@ static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
memset(&rsp, 0, sizeof(rsp));

err = hci_check_conn_params(min, max, latency, to_multiplier);
+ if (err) {
+ BT_WARN("Invalid conn params min 0x%4.4x max 0x%4.4x latency: 0x%4.4x TO: 0x%4.4x",
+ min, max, latency, to_multiplier);
+
+ err = hci_check_conn_params_legacy(min, max, latency,
+ to_multiplier);
+ if (!err) {
+ /* latency is invalid, cap it to the max allowed */
+ latency = min(499, (to_multiplier * 4 / max) - 1);
+ }
+ }
+
if (err)
rsp.result = cpu_to_le16(L2CAP_CONN_PARAM_REJECTED);
else
--
2.30.0.478.g8a0d178c01-goog


2021-02-17 15:17:51

by bluez.test.bot

[permalink] [raw]
Subject: RE: [v1] Bluetooth: Tolerate valid pre 4.2 conn params

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

---Test result---

##############################
Test: CheckPatch - PASS


##############################
Test: CheckGitLint - PASS


##############################
Test: CheckBuildK - PASS


##############################
Test: CheckTestRunner: Setup - PASS


##############################
Test: CheckTestRunner: l2cap-tester - PASS
Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6

##############################
Test: CheckTestRunner: bnep-tester - PASS
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: mgmt-tester - PASS
Total: 416, Passed: 402 (96.6%), Failed: 0, Not Run: 14

##############################
Test: CheckTestRunner: rfcomm-tester - PASS
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: sco-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: smp-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: userchan-tester - PASS
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth


Attachments:
l2cap-tester.log (42.33 kB)
bnep-tester.log (3.45 kB)
mgmt-tester.log (533.87 kB)
rfcomm-tester.log (11.38 kB)
sco-tester.log (9.66 kB)
smp-tester.log (11.52 kB)
userchan-tester.log (5.30 kB)
Download all attachments

2021-02-18 20:10:56

by Alain Michaud

[permalink] [raw]
Subject: Re: [v1] Bluetooth: Tolerate valid pre 4.2 conn params

+Marcel for viz


On Wed, Feb 17, 2021 at 10:11 AM <[email protected]> wrote:
>
> 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=434523
>
> ---Test result---
>
> ##############################
> Test: CheckPatch - PASS
>
>
> ##############################
> Test: CheckGitLint - PASS
>
>
> ##############################
> Test: CheckBuildK - PASS
>
>
> ##############################
> Test: CheckTestRunner: Setup - PASS
>
>
> ##############################
> Test: CheckTestRunner: l2cap-tester - PASS
> Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6
>
> ##############################
> Test: CheckTestRunner: bnep-tester - PASS
> Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
>
> ##############################
> Test: CheckTestRunner: mgmt-tester - PASS
> Total: 416, Passed: 402 (96.6%), Failed: 0, Not Run: 14
>
> ##############################
> Test: CheckTestRunner: rfcomm-tester - PASS
> Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
>
> ##############################
> Test: CheckTestRunner: sco-tester - PASS
> Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
>
> ##############################
> Test: CheckTestRunner: smp-tester - PASS
> Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
>
> ##############################
> Test: CheckTestRunner: userchan-tester - PASS
> Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
>
>
>
> ---
> Regards,
> Linux Bluetooth
>

2021-02-24 19:04:27

by Alain Michaud

[permalink] [raw]
Subject: Re: [v1] Bluetooth: Tolerate valid pre 4.2 conn params

Friendly ping.


On Thu, Feb 18, 2021 at 3:06 PM Alain Michaud <[email protected]> wrote:
>
> +Marcel for viz
>
>
> On Wed, Feb 17, 2021 at 10:11 AM <[email protected]> wrote:
> >
> > 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=434523
> >
> > ---Test result---
> >
> > ##############################
> > Test: CheckPatch - PASS
> >
> >
> > ##############################
> > Test: CheckGitLint - PASS
> >
> >
> > ##############################
> > Test: CheckBuildK - PASS
> >
> >
> > ##############################
> > Test: CheckTestRunner: Setup - PASS
> >
> >
> > ##############################
> > Test: CheckTestRunner: l2cap-tester - PASS
> > Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6
> >
> > ##############################
> > Test: CheckTestRunner: bnep-tester - PASS
> > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
> >
> > ##############################
> > Test: CheckTestRunner: mgmt-tester - PASS
> > Total: 416, Passed: 402 (96.6%), Failed: 0, Not Run: 14
> >
> > ##############################
> > Test: CheckTestRunner: rfcomm-tester - PASS
> > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
> >
> > ##############################
> > Test: CheckTestRunner: sco-tester - PASS
> > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
> >
> > ##############################
> > Test: CheckTestRunner: smp-tester - PASS
> > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
> >
> > ##############################
> > Test: CheckTestRunner: userchan-tester - PASS
> > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
> >
> >
> >
> > ---
> > Regards,
> > Linux Bluetooth
> >

2021-02-26 20:25:24

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v1] Bluetooth: Tolerate valid pre 4.2 conn params

Hi Alain,

> This patch tolerates connection parameters that were valid before
> ESR08_v1.0.0 which was also incoporated into the 4.2 core specification.
>
> In particular, this addresses compatibility issues with the Lenovo Yoga
> Mouse that sees its connection parameters rejected by the max_latency
> computation in hci_conn_check_params, but was perfectly valid at the
> time this mouse was qualified.
>
> Before the patch, the mouse was extremely laggy, after the patch, the
> mouse worked as expected.
>
> Signed-off-by: Alain Michaud <[email protected]>
> Reviewed-by: Miao-chen Chou <[email protected]>
> Tested-by: Harry Cutts <[email protected]>
>
> ---
>
> include/net/bluetooth/hci_core.h | 30 ++++++++++++++++++++++++++++++
> net/bluetooth/l2cap_core.c | 12 ++++++++++++
> 2 files changed, 42 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ebdd4afe30d2..67b75077c82e 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1646,6 +1646,36 @@ static inline struct smp_irk *hci_get_irk(struct hci_dev *hdev,
> return hci_find_irk_by_rpa(hdev, bdaddr);
> }
>
> +/* Erratum 5412 which has been fixed in 4.2 changed the validation of
> + * connection parameters. For backwards compatibility reasons, the old
> + * calculation must be tolerated.
> + * For further details :
> + * https://www.bluetooth.org/errata/errata_view.cfm?errata_id=5419
> + */
> +static inline int hci_check_conn_params_legacy(u16 min, u16 max, u16 latency,
> + u16 to_multiplier)

I would not bother with “inline” here. Let the compiler decide.

> +{
> + u16 max_latency;
> +
> + if (min > max || min < 6 || max > 3200)
> + return -EINVAL;
> +
> + if (to_multiplier < 10 || to_multiplier > 3200)
> + return -EINVAL;
> +
> + if (max >= to_multiplier * 8)
> + return -EINVAL;
> +
> + max_latency = (to_multiplier * 8 / max) - 1;
> + if (latency > 499 || latency > max_latency)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/* Connection Parameter Validation Helper.
> + * See Vol 6, Part B, section 4.5.1.
> + */
> static inline int hci_check_conn_params(u16 min, u16 max, u16 latency,
> u16 to_multiplier)
> {
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 72c2f5226d67..726e78bd85ff 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -5553,6 +5553,18 @@ static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
> memset(&rsp, 0, sizeof(rsp));
>
> err = hci_check_conn_params(min, max, latency, to_multiplier);
> + if (err) {
> + BT_WARN("Invalid conn params min 0x%4.4x max 0x%4.4x latency: 0x%4.4x TO: 0x%4.4x",
> + min, max, latency, to_multiplier);

Can we use bt_dev_warn() here?

> +
> + err = hci_check_conn_params_legacy(min, max, latency,
> + to_multiplier);
> + if (!err) {
> + /* latency is invalid, cap it to the max allowed */
> + latency = min(499, (to_multiplier * 4 / max) - 1);
> + }
> + }
> +
> if (err)
> rsp.result = cpu_to_le16(L2CAP_CONN_PARAM_REJECTED);
> else

Regards

Marcel