[Bluez PATCH v3 1/3] Listen and process remote name resolving failure

[Bluez PATCH v3 1/3] Listen and process remote name resolving failure

[Archie Pusaka]

From: Archie Pusaka <apusaka@chromium.org>

When Remote Name Resolve ends with failure, record this occurrence and
prevent remote name resolving for the same device for some time.
Increase the time duration for subsequent failures.
---
Hi maintainers,

This is the patch series for remote name request as was discussed here.
https://patchwork.kernel.org/project/bluetooth/patch/20211028191805.1.I35b7f3a496f834de6b43a32f94b6160cb1467c94@changeid/
Please also review the corresponding kernel space change.

Changes in v3:
* Rename MGMT const to align with the doc

Changes in v2:
* Stay silent instead of sending MGMT_OP_CONFIRM_NAME with DONT_CARE flag.

 lib/mgmt.h    |  1 +
 src/adapter.c | 15 +++++++++++++--
 src/device.c  | 23 +++++++++++++++++++++++
 src/device.h  |  2 ++
 4 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/lib/mgmt.h b/lib/mgmt.h
index 0d1678f01d..d860b27401 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -856,6 +856,7 @@ struct mgmt_ev_auth_failed {
 #define MGMT_DEV_FOUND_CONFIRM_NAME	0x01
 #define MGMT_DEV_FOUND_LEGACY_PAIRING	0x02
 #define MGMT_DEV_FOUND_NOT_CONNECTABLE	0x04
+#define MGMT_DEV_FOUND_NAME_REQUEST_FAILED 0x10
 
 #define MGMT_EV_DEVICE_FOUND		0x0012
 struct mgmt_ev_device_found {
diff --git a/src/adapter.c b/src/adapter.c
index d0d38621b8..6100448b5f 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -6989,6 +6989,7 @@ static void update_found_devices(struct btd_adapter *adapter,
 					uint8_t bdaddr_type, int8_t rssi,
 					bool confirm, bool legacy,
 					bool not_connectable,
+					bool name_resolve_failed,
 					const uint8_t *data, uint8_t data_len)
 {
 	struct btd_device *dev;
@@ -7081,6 +7082,9 @@ static void update_found_devices(struct btd_adapter *adapter,
 
 	device_set_legacy(dev, legacy);
 
+	if (name_resolve_failed)
+		device_name_resolve_fail(dev);
+
 	if (adapter->filtered_discovery)
 		device_set_rssi_with_delta(dev, rssi, 0);
 	else
@@ -7151,7 +7155,10 @@ static void update_found_devices(struct btd_adapter *adapter,
 	if (g_slist_find(adapter->discovery_found, dev))
 		return;
 
-	if (confirm)
+	/* If name is unknown but it's not allowed to resolve, don't send
+	 * MGMT_OP_CONFIRM_NAME.
+	 */
+	if (confirm && (name_known || device_name_resolve_allowed(dev)))
 		confirm_name(adapter, bdaddr, bdaddr_type, name_known);
 
 	adapter->discovery_found = g_slist_prepend(adapter->discovery_found,
@@ -7201,6 +7208,8 @@ static void device_found_callback(uint16_t index, uint16_t length,
 	uint32_t flags;
 	bool confirm_name;
 	bool legacy;
+	bool not_connectable;
+	bool name_resolve_failed;
 	char addr[18];
 
 	if (length < sizeof(*ev)) {
@@ -7230,10 +7239,12 @@ static void device_found_callback(uint16_t index, uint16_t length,
 
 	confirm_name = (flags & MGMT_DEV_FOUND_CONFIRM_NAME);
 	legacy = (flags & MGMT_DEV_FOUND_LEGACY_PAIRING);
+	not_connectable = (flags & MGMT_DEV_FOUND_NOT_CONNECTABLE);
+	name_resolve_failed = (flags & MGMT_DEV_FOUND_NAME_REQUEST_FAILED);
 
 	update_found_devices(adapter, &ev->addr.bdaddr, ev->addr.type,
 					ev->rssi, confirm_name, legacy,
-					flags & MGMT_DEV_FOUND_NOT_CONNECTABLE,
+					not_connectable, name_resolve_failed,
 					eir, eir_len);
 }
 
diff --git a/src/device.c b/src/device.c
index fdc2d50a47..699faeba3b 100644
--- a/src/device.c
+++ b/src/device.c
@@ -79,6 +79,8 @@
 #define GATT_INCLUDE_UUID_STR "2802"
 #define GATT_CHARAC_UUID_STR "2803"
 
+#define NAME_RESOLVE_RETRY_DELAY	120 /* seconds */
+
 static DBusConnection *dbus_conn = NULL;
 static unsigned service_state_cb_id;
 
@@ -272,6 +274,9 @@ struct btd_device {
 
 	GIOChannel	*att_io;
 	guint		store_id;
+
+	time_t		name_resolve_earliest_allow_time;
+	uint8_t		name_resolve_fail_count;
 };
 
 static const uint16_t uuid_list[] = {
@@ -4361,6 +4366,24 @@ bool device_name_known(struct btd_device *device)
 	return device->name[0] != '\0';
 }
 
+bool device_name_resolve_allowed(struct btd_device *device)
+{
+	return time(NULL) >= device->name_resolve_earliest_allow_time;
+}
+
+void device_name_resolve_fail(struct btd_device *device)
+{
+	if (!device)
+		return;
+
+	/* Punish this device by not allowing name resolve for some time.
+	 * increase punishment time for subsequent failures.
+	 */
+	device->name_resolve_fail_count++;
+	device->name_resolve_earliest_allow_time = time(NULL) +
+		NAME_RESOLVE_RETRY_DELAY * device->name_resolve_fail_count;
+}
+
 void device_set_class(struct btd_device *device, uint32_t class)
 {
 	if (device->class == class)
diff --git a/src/device.h b/src/device.h
index 5f615cb4b6..76d79855f8 100644
--- a/src/device.h
+++ b/src/device.h
@@ -25,6 +25,8 @@ void btd_device_device_set_name(struct btd_device *device, const char *name);
 void device_store_cached_name(struct btd_device *dev, const char *name);
 void device_get_name(struct btd_device *device, char *name, size_t len);
 bool device_name_known(struct btd_device *device);
+bool device_name_resolve_allowed(struct btd_device *device);
+void device_name_resolve_fail(struct btd_device *device);
 void device_set_class(struct btd_device *device, uint32_t class);
 void device_update_addr(struct btd_device *device, const bdaddr_t *bdaddr,
 							uint8_t bdaddr_type);
-- 
2.34.0.rc1.387.gb447b232ab-goog

[Bluez PATCH v3 2/3] device: Save remote name request attempts into cache file

[Archie Pusaka]

From: Archie Pusaka <apusaka@chromium.org>

Since a peer device is potentially removed if not discovered for more
than 30 seconds, we would lost the remote name request activity when
the device is rediscovered. This could end up with a remote name
request much sooner than we intend it to be.

Therefore, put the RNR record into a cache file, so we can recover it
when the peer device is rediscovered.

Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

(no changes since v1)

 src/device.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 94 insertions(+), 2 deletions(-)

diff --git a/src/device.c b/src/device.c
index 699faeba3b..fa6e3abc37 100644
--- a/src/device.c
+++ b/src/device.c
@@ -568,6 +568,63 @@ void device_store_cached_name(struct btd_device *dev, const char *name)
 	g_key_file_free(key_file);
 }
 
+static void device_store_cached_name_resolve_attempts(struct btd_device *dev)
+{
+	char filename[PATH_MAX];
+	char d_addr[18];
+	GKeyFile *key_file;
+	GError *gerr = NULL;
+	char *data;
+	char *data_old;
+	gsize length = 0;
+	gsize length_old = 0;
+	uint64_t earliest_allowed;
+	unsigned int num_failures;
+
+	if (device_address_is_private(dev)) {
+		DBG("Can't store name resolve for private addressed device %s",
+								dev->path);
+		return;
+	}
+
+	ba2str(&dev->bdaddr, d_addr);
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
+			btd_adapter_get_storage_dir(dev->adapter), d_addr);
+	create_file(filename, 0600);
+
+	key_file = g_key_file_new();
+	if (!g_key_file_load_from_file(key_file, filename, 0, &gerr)) {
+		error("Unable to load key file from %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
+
+	earliest_allowed = (uint64_t) dev->name_resolve_earliest_allow_time;
+	num_failures = dev->name_resolve_fail_count;
+
+	data_old = g_key_file_to_data(key_file, &length_old, NULL);
+
+	g_key_file_set_uint64(key_file, "NameResolve", "EarliestAllowed",
+							earliest_allowed);
+	g_key_file_set_integer(key_file, "NameResolve", "NumFailures",
+							num_failures);
+
+	data = g_key_file_to_data(key_file, &length, NULL);
+
+	if ((length != length_old) || (memcmp(data, data_old, length))) {
+		if (!g_file_set_contents(filename, data, length, &gerr)) {
+			error("Unable set contents for %s: (%s)", filename,
+								gerr->message);
+			g_error_free(gerr);
+		}
+	}
+
+	g_free(data);
+	g_free(data_old);
+
+	g_key_file_free(key_file);
+}
+
 static void browse_request_free(struct browse_req *req)
 {
 	struct btd_device *device = req->device;
@@ -3277,6 +3334,36 @@ failed:
 	return str;
 }
 
+static void load_cached_name_resolve_attempts(struct btd_device *device,
+					const char *local, const char *peer)
+{
+	char filename[PATH_MAX];
+	GKeyFile *key_file;
+	uint64_t earliest_allowed;
+	unsigned int num_failures;
+
+	if (device_address_is_private(device))
+		return;
+
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s", local, peer);
+
+	key_file = g_key_file_new();
+
+	if (!g_key_file_load_from_file(key_file, filename, 0, NULL))
+		goto failed;
+
+	earliest_allowed = g_key_file_get_uint64(key_file, "NameResolve",
+						"EarliestAllowed", NULL);
+	num_failures = g_key_file_get_uint64(key_file, "NameResolve",
+						"NumFailures", NULL);
+
+	device->name_resolve_earliest_allow_time = earliest_allowed;
+	device->name_resolve_fail_count = (uint8_t) num_failures;
+
+failed:
+	g_key_file_free(key_file);
+}
+
 static struct csrk_info *load_csrk(GKeyFile *key_file, const char *group)
 {
 	struct csrk_info *csrk;
@@ -4284,6 +4371,7 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 	struct btd_device *device;
 	char dst[18];
 	char *str;
+	const char *storage_dir;
 
 	ba2str(bdaddr, dst);
 	DBG("dst %s", dst);
@@ -4299,13 +4387,15 @@ struct btd_device *device_create(struct btd_adapter *adapter,
 	else
 		device->le = true;
 
-	str = load_cached_name(device, btd_adapter_get_storage_dir(adapter),
-									dst);
+	storage_dir = btd_adapter_get_storage_dir(adapter);
+	str = load_cached_name(device, storage_dir, dst);
 	if (str) {
 		strcpy(device->name, str);
 		g_free(str);
 	}
 
+	load_cached_name_resolve_attempts(device, storage_dir, dst);
+
 	return device;
 }
 
@@ -4382,6 +4472,8 @@ void device_name_resolve_fail(struct btd_device *device)
 	device->name_resolve_fail_count++;
 	device->name_resolve_earliest_allow_time = time(NULL) +
 		NAME_RESOLVE_RETRY_DELAY * device->name_resolve_fail_count;
+
+	device_store_cached_name_resolve_attempts(device);
 }
 
 void device_set_class(struct btd_device *device, uint32_t class)
-- 
2.34.0.rc1.387.gb447b232ab-goog

[Bluez PATCH v3 3/3] doc: Add Name Request Fail flag in device found event

[Archie Pusaka]

From: Archie Pusaka <apusaka@chromium.org>

Userspace should use this new flag to decide whether to do the remote
name resolving or not.
---

Changes in v3:
* Update the flag name to be more inlined with the spec.

Changes in v2:
* Update docs to reflect not sending DONT_CARE flag behavior.

 doc/mgmt-api.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index 97d33e30a1..845031b017 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -4089,6 +4089,7 @@ Device Connected Event
 		1	Legacy Pairing
 		2	Reserved (not in use)
 		3	Initiated Connection
+		4	Reserved (not in use)
 
 
 Device Disconnected Event
@@ -4263,6 +4264,7 @@ Device Found Event
 		1	Legacy Pairing
 		2	Not Connectable
 		3	Reserved (not in use)
+		4	Name Request Failed
 
 	For the RSSI field a value of 127 indicates that the RSSI is
 	not available. That can happen with Bluetooth 1.1 and earlier
@@ -4285,6 +4287,11 @@ Device Found Event
 	accept any connections. This can be indicated by Low Energy
 	devices that are in broadcaster role.
 
+	The Name Request Failed flag indicates that name resolving
+	procedure has ended with failure for this device. The user space
+	should use this information to determine when is a good time to
+	retry the name resolving procedure.
+
 
 Discovering Event
 =================
-- 
2.34.0.rc1.387.gb447b232ab-goog

RE: [Bluez,v3,1/3] Listen and process remote name resolving failure

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]

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

---Test result---

Test Summary:
CheckPatch                    PASS      4.30 seconds
GitLint                       FAIL      2.78 seconds
Prep - Setup ELL              PASS      42.35 seconds
Build - Prep                  PASS      0.46 seconds
Build - Configure             PASS      7.87 seconds
Build - Make                  PASS      179.42 seconds
Make Check                    PASS      9.71 seconds
Make Distcheck                PASS      220.26 seconds
Build w/ext ELL - Configure   PASS      8.00 seconds
Build w/ext ELL - Make        PASS      170.25 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint with rule in .gitlint
Output:
[Bluez,v3,1/3] Listen and process remote name resolving failure
12: B1 Line exceeds max length (121>80): "https://patchwork.kernel.org/project/bluetooth/patch/20211028191805.1.I35b7f3a496f834de6b43a32f94b6160cb1467c94@changeid/"




---
Regards,
Linux Bluetooth

Re: [Bluez PATCH v3 1/3] Listen and process remote name resolving failure

[Luiz Augusto von Dentz]

Hi Archie,

On Mon, Nov 15, 2021 at 1:27 AM Archie Pusaka <apusaka@google.com> wrote:
>
> From: Archie Pusaka <apusaka@chromium.org>
>
> When Remote Name Resolve ends with failure, record this occurrence and
> prevent remote name resolving for the same device for some time.
> Increase the time duration for subsequent failures.
> ---
> Hi maintainers,
>
> This is the patch series for remote name request as was discussed here.
> https://patchwork.kernel.org/project/bluetooth/patch/20211028191805.1.I35b7f3a496f834de6b43a32f94b6160cb1467c94@changeid/
> Please also review the corresponding kernel space change.
>
> Changes in v3:
> * Rename MGMT const to align with the doc
>
> Changes in v2:
> * Stay silent instead of sending MGMT_OP_CONFIRM_NAME with DONT_CARE flag.
>
>  lib/mgmt.h    |  1 +
>  src/adapter.c | 15 +++++++++++++--
>  src/device.c  | 23 +++++++++++++++++++++++
>  src/device.h  |  2 ++
>  4 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/lib/mgmt.h b/lib/mgmt.h
> index 0d1678f01d..d860b27401 100644
> --- a/lib/mgmt.h
> +++ b/lib/mgmt.h
> @@ -856,6 +856,7 @@ struct mgmt_ev_auth_failed {
>  #define MGMT_DEV_FOUND_CONFIRM_NAME    0x01
>  #define MGMT_DEV_FOUND_LEGACY_PAIRING  0x02
>  #define MGMT_DEV_FOUND_NOT_CONNECTABLE 0x04
> +#define MGMT_DEV_FOUND_NAME_REQUEST_FAILED 0x10
>
>  #define MGMT_EV_DEVICE_FOUND           0x0012
>  struct mgmt_ev_device_found {
> diff --git a/src/adapter.c b/src/adapter.c
> index d0d38621b8..6100448b5f 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -6989,6 +6989,7 @@ static void update_found_devices(struct btd_adapter *adapter,
>                                         uint8_t bdaddr_type, int8_t rssi,
>                                         bool confirm, bool legacy,
>                                         bool not_connectable,
> +                                       bool name_resolve_failed,
>                                         const uint8_t *data, uint8_t data_len)
>  {
>         struct btd_device *dev;
> @@ -7081,6 +7082,9 @@ static void update_found_devices(struct btd_adapter *adapter,
>
>         device_set_legacy(dev, legacy);
>
> +       if (name_resolve_failed)
> +               device_name_resolve_fail(dev);
> +
>         if (adapter->filtered_discovery)
>                 device_set_rssi_with_delta(dev, rssi, 0);
>         else
> @@ -7151,7 +7155,10 @@ static void update_found_devices(struct btd_adapter *adapter,
>         if (g_slist_find(adapter->discovery_found, dev))
>                 return;
>
> -       if (confirm)
> +       /* If name is unknown but it's not allowed to resolve, don't send
> +        * MGMT_OP_CONFIRM_NAME.
> +        */
> +       if (confirm && (name_known || device_name_resolve_allowed(dev)))
>                 confirm_name(adapter, bdaddr, bdaddr_type, name_known);
>
>         adapter->discovery_found = g_slist_prepend(adapter->discovery_found,
> @@ -7201,6 +7208,8 @@ static void device_found_callback(uint16_t index, uint16_t length,
>         uint32_t flags;
>         bool confirm_name;
>         bool legacy;
> +       bool not_connectable;
> +       bool name_resolve_failed;
>         char addr[18];
>
>         if (length < sizeof(*ev)) {
> @@ -7230,10 +7239,12 @@ static void device_found_callback(uint16_t index, uint16_t length,
>
>         confirm_name = (flags & MGMT_DEV_FOUND_CONFIRM_NAME);
>         legacy = (flags & MGMT_DEV_FOUND_LEGACY_PAIRING);
> +       not_connectable = (flags & MGMT_DEV_FOUND_NOT_CONNECTABLE);
> +       name_resolve_failed = (flags & MGMT_DEV_FOUND_NAME_REQUEST_FAILED);
>
>         update_found_devices(adapter, &ev->addr.bdaddr, ev->addr.type,
>                                         ev->rssi, confirm_name, legacy,
> -                                       flags & MGMT_DEV_FOUND_NOT_CONNECTABLE,
> +                                       not_connectable, name_resolve_failed,
>                                         eir, eir_len);
>  }
>
> diff --git a/src/device.c b/src/device.c
> index fdc2d50a47..699faeba3b 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -79,6 +79,8 @@
>  #define GATT_INCLUDE_UUID_STR "2802"
>  #define GATT_CHARAC_UUID_STR "2803"
>
> +#define NAME_RESOLVE_RETRY_DELAY       120 /* seconds */

I'd make this configurable since on headless systems it might be a
good idea to have an option to disable the retrying logic.

>  static DBusConnection *dbus_conn = NULL;
>  static unsigned service_state_cb_id;
>
> @@ -272,6 +274,9 @@ struct btd_device {
>
>         GIOChannel      *att_io;
>         guint           store_id;
> +
> +       time_t          name_resolve_earliest_allow_time;
> +       uint8_t         name_resolve_fail_count;
>  };
>
>  static const uint16_t uuid_list[] = {
> @@ -4361,6 +4366,24 @@ bool device_name_known(struct btd_device *device)
>         return device->name[0] != '\0';
>  }
>
> +bool device_name_resolve_allowed(struct btd_device *device)
> +{
> +       return time(NULL) >= device->name_resolve_earliest_allow_time;
> +}
> +
> +void device_name_resolve_fail(struct btd_device *device)
> +{
> +       if (!device)
> +               return;
> +
> +       /* Punish this device by not allowing name resolve for some time.
> +        * increase punishment time for subsequent failures.
> +        */
> +       device->name_resolve_fail_count++;
> +       device->name_resolve_earliest_allow_time = time(NULL) +
> +               NAME_RESOLVE_RETRY_DELAY * device->name_resolve_fail_count;

Like I said above we should probably make the number of retries and
intervals configurable, have a look how it was done for reconnections
in the policy plugin since I believe this would look very similar to
that. Anyway we can't really use the system time as that can be
modified causing jumps backward or forward in time so you must use
CLOCK_MONOTONIC if you don't want it to be affected by system time
changes.

The other possible solution would be not to have any retry logic since
those device are likely to fail resolving their names on each retry,
or we are doing this because we now properly abort the name
resolution?

> +}
> +
>  void device_set_class(struct btd_device *device, uint32_t class)
>  {
>         if (device->class == class)
> diff --git a/src/device.h b/src/device.h
> index 5f615cb4b6..76d79855f8 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -25,6 +25,8 @@ void btd_device_device_set_name(struct btd_device *device, const char *name);
>  void device_store_cached_name(struct btd_device *dev, const char *name);
>  void device_get_name(struct btd_device *device, char *name, size_t len);
>  bool device_name_known(struct btd_device *device);
> +bool device_name_resolve_allowed(struct btd_device *device);
> +void device_name_resolve_fail(struct btd_device *device);
>  void device_set_class(struct btd_device *device, uint32_t class);
>  void device_update_addr(struct btd_device *device, const bdaddr_t *bdaddr,
>                                                         uint8_t bdaddr_type);
> --
> 2.34.0.rc1.387.gb447b232ab-goog
>


-- 
Luiz Augusto von Dentz

Re: [Bluez PATCH v3 1/3] Listen and process remote name resolving failure

[Archie Pusaka]

Hi Luiz,

On Tue, 16 Nov 2021 at 07:50, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Archie,
>
> On Mon, Nov 15, 2021 at 1:27 AM Archie Pusaka <apusaka@google.com> wrote:
> >
> > From: Archie Pusaka <apusaka@chromium.org>
> >
> > When Remote Name Resolve ends with failure, record this occurrence and
> > prevent remote name resolving for the same device for some time.
> > Increase the time duration for subsequent failures.
> > ---
> > Hi maintainers,
> >
> > This is the patch series for remote name request as was discussed here.
> > https://patchwork.kernel.org/project/bluetooth/patch/20211028191805.1.I35b7f3a496f834de6b43a32f94b6160cb1467c94@changeid/
> > Please also review the corresponding kernel space change.
> >
> > Changes in v3:
> > * Rename MGMT const to align with the doc
> >
> > Changes in v2:
> > * Stay silent instead of sending MGMT_OP_CONFIRM_NAME with DONT_CARE flag.
> >
> >  lib/mgmt.h    |  1 +
> >  src/adapter.c | 15 +++++++++++++--
> >  src/device.c  | 23 +++++++++++++++++++++++
> >  src/device.h  |  2 ++
> >  4 files changed, 39 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/mgmt.h b/lib/mgmt.h
> > index 0d1678f01d..d860b27401 100644
> > --- a/lib/mgmt.h
> > +++ b/lib/mgmt.h
> > @@ -856,6 +856,7 @@ struct mgmt_ev_auth_failed {
> >  #define MGMT_DEV_FOUND_CONFIRM_NAME    0x01
> >  #define MGMT_DEV_FOUND_LEGACY_PAIRING  0x02
> >  #define MGMT_DEV_FOUND_NOT_CONNECTABLE 0x04
> > +#define MGMT_DEV_FOUND_NAME_REQUEST_FAILED 0x10
> >
> >  #define MGMT_EV_DEVICE_FOUND           0x0012
> >  struct mgmt_ev_device_found {
> > diff --git a/src/adapter.c b/src/adapter.c
> > index d0d38621b8..6100448b5f 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -6989,6 +6989,7 @@ static void update_found_devices(struct btd_adapter *adapter,
> >                                         uint8_t bdaddr_type, int8_t rssi,
> >                                         bool confirm, bool legacy,
> >                                         bool not_connectable,
> > +                                       bool name_resolve_failed,
> >                                         const uint8_t *data, uint8_t data_len)
> >  {
> >         struct btd_device *dev;
> > @@ -7081,6 +7082,9 @@ static void update_found_devices(struct btd_adapter *adapter,
> >
> >         device_set_legacy(dev, legacy);
> >
> > +       if (name_resolve_failed)
> > +               device_name_resolve_fail(dev);
> > +
> >         if (adapter->filtered_discovery)
> >                 device_set_rssi_with_delta(dev, rssi, 0);
> >         else
> > @@ -7151,7 +7155,10 @@ static void update_found_devices(struct btd_adapter *adapter,
> >         if (g_slist_find(adapter->discovery_found, dev))
> >                 return;
> >
> > -       if (confirm)
> > +       /* If name is unknown but it's not allowed to resolve, don't send
> > +        * MGMT_OP_CONFIRM_NAME.
> > +        */
> > +       if (confirm && (name_known || device_name_resolve_allowed(dev)))
> >                 confirm_name(adapter, bdaddr, bdaddr_type, name_known);
> >
> >         adapter->discovery_found = g_slist_prepend(adapter->discovery_found,
> > @@ -7201,6 +7208,8 @@ static void device_found_callback(uint16_t index, uint16_t length,
> >         uint32_t flags;
> >         bool confirm_name;
> >         bool legacy;
> > +       bool not_connectable;
> > +       bool name_resolve_failed;
> >         char addr[18];
> >
> >         if (length < sizeof(*ev)) {
> > @@ -7230,10 +7239,12 @@ static void device_found_callback(uint16_t index, uint16_t length,
> >
> >         confirm_name = (flags & MGMT_DEV_FOUND_CONFIRM_NAME);
> >         legacy = (flags & MGMT_DEV_FOUND_LEGACY_PAIRING);
> > +       not_connectable = (flags & MGMT_DEV_FOUND_NOT_CONNECTABLE);
> > +       name_resolve_failed = (flags & MGMT_DEV_FOUND_NAME_REQUEST_FAILED);
> >
> >         update_found_devices(adapter, &ev->addr.bdaddr, ev->addr.type,
> >                                         ev->rssi, confirm_name, legacy,
> > -                                       flags & MGMT_DEV_FOUND_NOT_CONNECTABLE,
> > +                                       not_connectable, name_resolve_failed,
> >                                         eir, eir_len);
> >  }
> >
> > diff --git a/src/device.c b/src/device.c
> > index fdc2d50a47..699faeba3b 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -79,6 +79,8 @@
> >  #define GATT_INCLUDE_UUID_STR "2802"
> >  #define GATT_CHARAC_UUID_STR "2803"
> >
> > +#define NAME_RESOLVE_RETRY_DELAY       120 /* seconds */
>
> I'd make this configurable since on headless systems it might be a
> good idea to have an option to disable the retrying logic.
>
Ack. Will be revised.

> >  static DBusConnection *dbus_conn = NULL;
> >  static unsigned service_state_cb_id;
> >
> > @@ -272,6 +274,9 @@ struct btd_device {
> >
> >         GIOChannel      *att_io;
> >         guint           store_id;
> > +
> > +       time_t          name_resolve_earliest_allow_time;
> > +       uint8_t         name_resolve_fail_count;
> >  };
> >
> >  static const uint16_t uuid_list[] = {
> > @@ -4361,6 +4366,24 @@ bool device_name_known(struct btd_device *device)
> >         return device->name[0] != '\0';
> >  }
> >
> > +bool device_name_resolve_allowed(struct btd_device *device)
> > +{
> > +       return time(NULL) >= device->name_resolve_earliest_allow_time;
> > +}
> > +
> > +void device_name_resolve_fail(struct btd_device *device)
> > +{
> > +       if (!device)
> > +               return;
> > +
> > +       /* Punish this device by not allowing name resolve for some time.
> > +        * increase punishment time for subsequent failures.
> > +        */
> > +       device->name_resolve_fail_count++;
> > +       device->name_resolve_earliest_allow_time = time(NULL) +
> > +               NAME_RESOLVE_RETRY_DELAY * device->name_resolve_fail_count;
>
> Like I said above we should probably make the number of retries and
> intervals configurable, have a look how it was done for reconnections
> in the policy plugin since I believe this would look very similar to
> that. Anyway we can't really use the system time as that can be
> modified causing jumps backward or forward in time so you must use
> CLOCK_MONOTONIC if you don't want it to be affected by system time
> changes.
>
Good point! Will be revised.

> The other possible solution would be not to have any retry logic since
> those device are likely to fail resolving their names on each retry,
> or we are doing this because we now properly abort the name
> resolution?
>
This might also be caused by the peer device suddenly losing power or
moving away, therefore is unresponsive to the name resolve request. As
such, I think it's important to retry at some point.

> > +}
> > +
> >  void device_set_class(struct btd_device *device, uint32_t class)
> >  {
> >         if (device->class == class)
> > diff --git a/src/device.h b/src/device.h
> > index 5f615cb4b6..76d79855f8 100644
> > --- a/src/device.h
> > +++ b/src/device.h
> > @@ -25,6 +25,8 @@ void btd_device_device_set_name(struct btd_device *device, const char *name);
> >  void device_store_cached_name(struct btd_device *dev, const char *name);
> >  void device_get_name(struct btd_device *device, char *name, size_t len);
> >  bool device_name_known(struct btd_device *device);
> > +bool device_name_resolve_allowed(struct btd_device *device);
> > +void device_name_resolve_fail(struct btd_device *device);
> >  void device_set_class(struct btd_device *device, uint32_t class);
> >  void device_update_addr(struct btd_device *device, const bdaddr_t *bdaddr,
> >                                                         uint8_t bdaddr_type);
> > --
> > 2.34.0.rc1.387.gb447b232ab-goog
> >
>
>
> --
> Luiz Augusto von Dentz

Thanks,
Archie