Skip to contentSkip to conversation

device: fix advertising data UUIDs ignored when Cache = yes

PATCH Applied David Lechner 4 participants 7 messages Aug 25, 2021 – Aug 26, 2021 Bluetooth drivers

lore
Share & tools
lore
#0 lore raw

v2 cover device: fix advertising data UUIDs ignored when Cache = yes

When [GATT] Cache = yes is set in configuration BlueZ does not use cached UUIDs for the list of service UUIDs. However, it doesn't clear the in-memory list of UUIDs received from advertising data, so when a device is scanned, connected and disconnected, the internal state still reflects that the UUIDs from the advertising data have already been handled. device_add_eir_uuids() ignored the UUIDs from the advertising data because both dev->le_state.svc_resolved == true and dev->eir_uuids still contains the UUIDs from the previous scan session.

v2 changes:
* split into two patches
* always clear eir_uuids list on disconnect
* move le_state.svc_resolved = false to gatt_cache_cleanup()

Issue: https://github.com/bluez/bluez/issues/192

David Lechner (2):
device: clear eir_uuids list on disconnect
device: set le_state.svc_resolved = false in gatt_cache_cleanup()

 src/device.c | 4 ++++
 1 file changed, 4 insertions(+)
Signature · 1 line
2.25.1
#1 lore raw

PATCH v2 1/2 device: clear eir_uuids list on disconnect

The eir_uuids list contains GATT service UUIDs from advertising data. The device may advertise different UUIDs each time it is scanned and connected, so the list needs to be cleared when the device disconnects.

This partially fixes an issue where the UUIDs D-Bus property is empty after scanning, connecting, disconnecting and scanning again when [GATT] Cache = yes is set in main.conf.

Issue: https://github.com/bluez/bluez/issues/192

---
 src/device.c | 3 +++
 1 file changed, 3 insertions(+)
src/device.c +3 -0
index 807106812..53ef3e9a1 100644
@@ -3129,6 +3129,9 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type)
 
 	device_update_last_seen(device, bdaddr_type);
 
+	g_slist_free_full(device->eir_uuids, g_free);
+	device->eir_uuids = NULL;
+
 	g_dbus_emit_property_changed(dbus_conn, device->path,
 						DEVICE_INTERFACE, "Connected");
 
Signature · 1 line
2.25.1
#2 lore raw

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

---Test result---

Test Summary:

CheckPatch                    PASS      0.48 seconds
GitLint                       PASS      0.21 seconds
Prep - Setup ELL              PASS      40.13 seconds
Build - Prep                  PASS      0.09 seconds
Build - Configure             PASS      7.11 seconds
Build - Make                  PASS      174.78 seconds
Make Check                    PASS      8.99 seconds
Make Distcheck                PASS      206.59 seconds
Build w/ext ELL - Configure   PASS      7.11 seconds
Build w/ext ELL - Make        PASS      166.73 seconds

Details
##############################
Test: CheckPatch - PASS
Desc: Run checkpatch.pl script with rule in .checkpatch.conf

##############################
Test: GitLint - PASS
Desc: Run gitlint with rule in .gitlint

##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree

##############################
Test: Make Check - PASS
Desc: Run 'make check'

##############################
Test: Make Distcheck - PASS
Desc: Run distcheck to check the distribution

##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration

---
Regards,
Linux Bluetooth

maintainer Applied
#3 lore raw

Re: [PATCH BlueZ v2 1/2] device: clear eir_uuids list on disconnect

In reply to David Lechner

Hi David,

Quoted 30 lines · David Lechner

The eir_uuids list contains GATT service UUIDs from advertising data. The device may advertise different UUIDs each time it is scanned and connected, so the list needs to be cleared when the device disconnects.

This partially fixes an issue where the UUIDs D-Bus property is empty after scanning, connecting, disconnecting and scanning again when [GATT] Cache = yes is set in main.conf.

Issue: https://github.com/bluez/bluez/issues/192

---
 src/device.c | 3 +++
 1 file changed, 3 insertions(+)
src/device.c +3 -1
index 807106812..53ef3e9a1 100644
@@ -3129,6 +3129,9 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type)
 
        device_update_last_seen(device, bdaddr_type);
 
+       g_slist_free_full(device->eir_uuids, g_free);
+       device->eir_uuids = NULL;
+
        g_dbus_emit_property_changed(dbus_conn, device->path,
                                                DEVICE_INTERFACE, "Connected");
 
--
Quoted 1 line

2.25.1

Applied, thanks.

Signature · 1 line
Luiz Augusto von Dentz
#4 lore raw

AW: [PATCH BlueZ v2 1/2] device: clear eir_uuids list on disconnect

Hello,

I have just found the same problem even if [GATT] Cache = no is set in main.conf. It is simply the same because the device is held as temporary device.

Additionally the method "dev_property_get_uuids" in device.c only returns UUIDs of the cached (temporary hold) device because dev->le_state.svc_resolved is not turned to false on disconnect. So if a disconnected device advertised an new UUID, you never get notified about it through dbus.

Through DBus device node I would expect to get both UUID lists. The cached ones and also the advertised ones.
Actually I just merge the two lists and provide the result to DBus device->UUIDs request.

Hope you can change this.

Thanks.

Thomas

-----Ursprüngliche Nachricht-----
Von: Luiz Augusto von Dentz [mailto:[email protected]]
Gesendet: Donnerstag, 26. August 2021 00:13
An: David Lechner <[email protected]>
Cc: [email protected]
Betreff: Re: [PATCH BlueZ v2 1/2] device: clear eir_uuids list on disconnect

Hi David,

Quoted 31 lines · David Lechner

The eir_uuids list contains GATT service UUIDs from advertising data. The device may advertise different UUIDs each time it is scanned and connected, so the list needs to be cleared when the device disconnects.

This partially fixes an issue where the UUIDs D-Bus property is empty after scanning, connecting, disconnecting and scanning again when [GATT] Cache = yes is set in main.conf.

Issue: https://github.com/bluez/bluez/issues/192

---
 src/device.c | 3 +++
 1 file changed, 3 insertions(+)
src/device.c index 807106812..53ef3e9a1 renamed from src/device.c +0 -0

100644

src/device.c +3 -0
@@ -3129,6 +3129,9 @@ void device_remove_connection(struct btd_device 
 *device, uint8_t bdaddr_type)
 
        device_update_last_seen(device, bdaddr_type);
 
+       g_slist_free_full(device->eir_uuids, g_free);
+       device->eir_uuids = NULL;
+
        g_dbus_emit_property_changed(dbus_conn, device->path,
                                                DEVICE_INTERFACE, 

"Connected");

Quoted 2 lines

--
2.25.1

Applied, thanks.

--
Luiz Augusto von Dentz

#5 lore raw

Re: AW: [PATCH BlueZ v2 1/2] device: clear eir_uuids list on disconnect

Quoted 10 lines · Eisenkolb Thomas

Hello,

I have just found the same problem even if [GATT] Cache = no is set in main.conf. It is simply the same because the device is held as temporary device.

Additionally the method "dev_property_get_uuids" in device.c only returns UUIDs of the cached (temporary hold) device because dev->le_state.svc_resolved is not turned to false on disconnect. So if a disconnected device advertised an new UUID, you never get notified about it through dbus.

Quoted 6 lines

Through DBus device node I would expect to get both UUID lists. The cached ones and also the advertised ones.
Actually I just merge the two lists and provide the result to DBus device->UUIDs request.

Hope you can change this.

This series has already been applied, so it can't be changed.

If you are still having problems after this series, can you share some logs so that we can see exactly the problem?

#6 lore raw

PATCH v2 2/2 device: set le_state.svc_resolved = false in gatt_cache_cleanup()

In reply to David Lechner

When the GATT cache is cleared, there is no longer a list of GATT services, so we need to set le_state.svc_resolved = false so that the next time the device connects, it will enumerate the services again.

This partially fixes an issue where the UUIDs D-Bus property was empty after scanning, connecting, disconnecting and scanning again when [GATT] Cache = yes is set in main.conf.

Issue: https://github.com/bluez/bluez/issues/192

---
 src/device.c | 1 +
 1 file changed, 1 insertion(+)
src/device.c +1 -0
index 53ef3e9a1..6d534c488 100644
@@ -580,6 +580,7 @@ static void gatt_cache_cleanup(struct btd_device *device)
 
 	bt_gatt_client_cancel_all(device->client);
 	gatt_db_clear(device->db);
+	device->le_state.svc_resolved = false;
 }
 
 static void gatt_client_cleanup(struct btd_device *device)
Signature · 1 line
2.25.1