| Message ID | 20241028113643.1012491-1-quic_chejiang@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] device: only use the address type selection algorithm when remote device is a dual-mode device when pair device | expand |
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);