| Message ID | 20241028113643.1012491-1-quic_chejiang@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] device: only use the address type selection algorithm when remote device is a dual-mode device when pair device | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | fail | WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T1 Title exceeds max length (116>80): "[v1] device: only use the address type selection algorithm when remote device is a dual-mode device when pair device" |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
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=903776 ---Test result--- Test Summary: CheckPatch PASS 0.46 seconds GitLint FAIL 0.56 seconds BuildEll PASS 24.45 seconds BluezMake PASS 1660.51 seconds MakeCheck PASS 13.31 seconds MakeDistcheck PASS 178.89 seconds CheckValgrind PASS 253.46 seconds CheckSmatch PASS 358.61 seconds bluezmakeextell PASS 121.57 seconds IncrementalBuild PASS 1647.52 seconds ScanBuild PASS 1031.62 seconds Details ############################## Test: GitLint - FAIL Desc: Run gitlint Output: [v1] device: only use the address type selection algorithm when remote device is a dual-mode device when pair device WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T1 Title exceeds max length (116>80): "[v1] device: only use the address type selection algorithm when remote device is a dual-mode device when pair device" --- Regards, Linux Bluetooth
Hi Cheng, On Mon, Oct 28, 2024 at 7:37 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote: > > --- > src/device.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/src/device.c b/src/device.c > index 7585184de..71fdbb145 100644 > --- a/src/device.c > +++ b/src/device.c > @@ -3077,12 +3077,21 @@ static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg, > if (device->bonding) > return btd_error_in_progress(msg); > > - if (device->bredr_state.bonded) > + /* Only use this selection algorithms when device is combo > + * chip. Ohterwise, it will use the wrong bearer to establish > + * a connection if the device is already paired. which will > + * stall the pairing procedure. > + */ > + if (device->bredr && device->le) { > + if (device->bredr_state.bonded) > + bdaddr_type = device->bdaddr_type; > + else if (device->le_state.bonded) > + bdaddr_type = BDADDR_BREDR; > + else > + bdaddr_type = select_conn_bearer(device); > + } else { > bdaddr_type = device->bdaddr_type; > - else if (device->le_state.bonded) > - bdaddr_type = BDADDR_BREDR; > - else > - bdaddr_type = select_conn_bearer(device); > + } This seems weird without there being a bug with the state itself, for instance how would it select the wrong bearer if it is not supported? Also the lack of proper explanation in the commit message doesn't help to grasp what is going on here, so please have backtrace or something attached since we need to understand why it would be selecting the wrong bearer, or perhaps the bearer is being advertised as supported when in fact it isn't? > state = get_state(device, bdaddr_type); > > -- > 2.25.1 > >
Hi Luiz, I update a new patchset with more info. In the current logic of pair_device (https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/device.c#n3080). If a BLE-only device paired twice, in the second trial, it will select the bdaddr_type to BDADDR_BREDR since device->le_state.bonded is set. We should only use the following logic for a dual-mode remote. if (device->bredr_state.bonded) bdaddr_type = device->bdaddr_type; else if (device->le_state.bonded) bdaddr_type = BDADDR_BREDR; else bdaddr_type = select_conn_bearer(device); state = get_state(device, bdaddr_type); On 10/28/2024 10:13 PM, Luiz Augusto von Dentz wrote: > Hi Cheng, > > On Mon, Oct 28, 2024 at 7:37 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote: >> >> --- >> src/device.c | 19 ++++++++++++++----- >> 1 file changed, 14 insertions(+), 5 deletions(-) >> >> diff --git a/src/device.c b/src/device.c >> index 7585184de..71fdbb145 100644 >> --- a/src/device.c >> +++ b/src/device.c >> @@ -3077,12 +3077,21 @@ static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg, >> if (device->bonding) >> return btd_error_in_progress(msg); >> >> - if (device->bredr_state.bonded) >> + /* Only use this selection algorithms when device is combo >> + * chip. Ohterwise, it will use the wrong bearer to establish >> + * a connection if the device is already paired. which will >> + * stall the pairing procedure. >> + */ >> + if (device->bredr && device->le) { >> + if (device->bredr_state.bonded) >> + bdaddr_type = device->bdaddr_type; >> + else if (device->le_state.bonded) >> + bdaddr_type = BDADDR_BREDR; >> + else >> + bdaddr_type = select_conn_bearer(device); >> + } else { >> bdaddr_type = device->bdaddr_type; >> - else if (device->le_state.bonded) >> - bdaddr_type = BDADDR_BREDR; >> - else >> - bdaddr_type = select_conn_bearer(device); >> + } > > This seems weird without there being a bug with the state itself, for > instance how would it select the wrong bearer if it is not supported? > Also the lack of proper explanation in the commit message doesn't help > to grasp what is going on here, so please have backtrace or something > attached since we need to understand why it would be selecting the > wrong bearer, or perhaps the bearer is being advertised as supported > when in fact it isn't? > >> state = get_state(device, bdaddr_type); >> >> -- >> 2.25.1 >> >> > >
diff --git a/src/device.c b/src/device.c index 7585184de..71fdbb145 100644 --- a/src/device.c +++ b/src/device.c @@ -3077,12 +3077,21 @@ static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg, if (device->bonding) return btd_error_in_progress(msg); - if (device->bredr_state.bonded) + /* Only use this selection algorithms when device is combo + * chip. Ohterwise, it will use the wrong bearer to establish + * a connection if the device is already paired. which will + * stall the pairing procedure. + */ + if (device->bredr && device->le) { + if (device->bredr_state.bonded) + bdaddr_type = device->bdaddr_type; + else if (device->le_state.bonded) + bdaddr_type = BDADDR_BREDR; + else + bdaddr_type = select_conn_bearer(device); + } else { bdaddr_type = device->bdaddr_type; - else if (device->le_state.bonded) - bdaddr_type = BDADDR_BREDR; - else - bdaddr_type = select_conn_bearer(device); + } state = get_state(device, bdaddr_type);