* [Bluez PATCH] device: Fix device can't be scanned for 5 mins after reboot
@ 2022-01-13 6:16 Archie Pusaka
2022-01-13 7:05 ` Marcel Holtmann
2022-01-13 7:31 ` [Bluez] " bluez.test.bot
0 siblings, 2 replies; 4+ messages in thread
From: Archie Pusaka @ 2022-01-13 6:16 UTC (permalink / raw)
To: linux-bluetooth, Luiz Augusto von Dentz
Cc: CrosBT Upstreaming, Archie Pusaka, Shuo-Peng Liao
From: Archie Pusaka <apusaka@chromium.org>
After the patches which limit the attempts of doing remote name
resolving, there's an issue which prevents BlueZ to RNR new devices
for 5 minutes after reboot. It's caused by failed_time is init to 0,
and is then treated as the timestamp when the device failed RNR.
However, actually there is no failure yet.
This CL fixes it by always allowing RNR when failed_time = 0.
Reviewed-by: Shuo-Peng Liao <deanliao@chromium.org>
---
src/device.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/device.c b/src/device.c
index f2447c478e..16b1ed5bd3 100644
--- a/src/device.c
+++ b/src/device.c
@@ -4487,10 +4487,15 @@ bool device_is_name_resolve_allowed(struct btd_device *device)
clock_gettime(CLOCK_MONOTONIC, &now);
- /* If now < failed_time, it means the clock has somehow turned back,
- * possibly because of system restart. Allow name request in this case.
+ /* Allow name request for any of these cases:
+ * (1) failed_time is empty (0). Meaning no prior failure.
+ * (2) now < failed_time. Meaning the clock has somehow turned back,
+ * possibly because of system restart. Allow just to be safe.
+ * (3) now >= failed_time + name_request_retry_delay. Meaning the
+ * period of not sending name request is over.
*/
- return now.tv_sec < device->name_resolve_failed_time ||
+ return device->name_resolve_failed_time == 0 ||
+ now.tv_sec < device->name_resolve_failed_time ||
now.tv_sec >= device->name_resolve_failed_time +
btd_opts.name_request_retry_delay;
}
--
2.34.1.575.g55b058a8bb-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Bluez PATCH] device: Fix device can't be scanned for 5 mins after reboot
2022-01-13 6:16 [Bluez PATCH] device: Fix device can't be scanned for 5 mins after reboot Archie Pusaka
@ 2022-01-13 7:05 ` Marcel Holtmann
2022-01-13 7:51 ` Archie Pusaka
2022-01-13 7:31 ` [Bluez] " bluez.test.bot
1 sibling, 1 reply; 4+ messages in thread
From: Marcel Holtmann @ 2022-01-13 7:05 UTC (permalink / raw)
To: Archie Pusaka
Cc: linux-bluetooth, Luiz Augusto von Dentz, CrosBT Upstreaming,
Archie Pusaka, Shuo-Peng Liao
Hi Archie,
> After the patches which limit the attempts of doing remote name
> resolving, there's an issue which prevents BlueZ to RNR new devices
> for 5 minutes after reboot. It's caused by failed_time is init to 0,
> and is then treated as the timestamp when the device failed RNR.
> However, actually there is no failure yet.
>
> This CL fixes it by always allowing RNR when failed_time = 0.
>
> Reviewed-by: Shuo-Peng Liao <deanliao@chromium.org>
> ---
>
> src/device.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index f2447c478e..16b1ed5bd3 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -4487,10 +4487,15 @@ bool device_is_name_resolve_allowed(struct btd_device *device)
>
> clock_gettime(CLOCK_MONOTONIC, &now);
>
> - /* If now < failed_time, it means the clock has somehow turned back,
> - * possibly because of system restart. Allow name request in this case.
> + /* Allow name request for any of these cases:
> + * (1) failed_time is empty (0). Meaning no prior failure.
> + * (2) now < failed_time. Meaning the clock has somehow turned back,
> + * possibly because of system restart. Allow just to be safe.
> + * (3) now >= failed_time + name_request_retry_delay. Meaning the
> + * period of not sending name request is over.
> */
> - return now.tv_sec < device->name_resolve_failed_time ||
> + return device->name_resolve_failed_time == 0 ||
> + now.tv_sec < device->name_resolve_failed_time ||
> now.tv_sec >= device->name_resolve_failed_time +
> btd_opts.name_request_retry_delay;
> }
I have the feeling this becomes complex and hard to follow. I appreciate the comment above, but just for thoughts, would it be better to do something like this:
/* failed_time is empty (0). Meaning no prior failure. */
if (device->name_..failed_time == 0)
return true;
..
return false;
Regards
Marcel
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [Bluez] device: Fix device can't be scanned for 5 mins after reboot
2022-01-13 6:16 [Bluez PATCH] device: Fix device can't be scanned for 5 mins after reboot Archie Pusaka
2022-01-13 7:05 ` Marcel Holtmann
@ 2022-01-13 7:31 ` bluez.test.bot
1 sibling, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2022-01-13 7:31 UTC (permalink / raw)
To: linux-bluetooth, apusaka
[-- Attachment #1: Type: text/plain, Size: 995 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=605086
---Test result---
Test Summary:
CheckPatch PASS 1.37 seconds
GitLint PASS 1.00 seconds
Prep - Setup ELL PASS 42.67 seconds
Build - Prep PASS 0.67 seconds
Build - Configure PASS 8.57 seconds
Build - Make PASS 1416.24 seconds
Make Check PASS 12.01 seconds
Make Check w/Valgrind PASS 446.97 seconds
Make Distcheck PASS 219.77 seconds
Build w/ext ELL - Configure PASS 8.66 seconds
Build w/ext ELL - Make PASS 1400.11 seconds
Incremental Build with patchesPASS 0.00 seconds
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Bluez PATCH] device: Fix device can't be scanned for 5 mins after reboot
2022-01-13 7:05 ` Marcel Holtmann
@ 2022-01-13 7:51 ` Archie Pusaka
0 siblings, 0 replies; 4+ messages in thread
From: Archie Pusaka @ 2022-01-13 7:51 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth, Luiz Augusto von Dentz, CrosBT Upstreaming,
Shuo-Peng Liao
Hi Marcel,
On Thu, 13 Jan 2022 at 15:05, Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Archie,
>
> > After the patches which limit the attempts of doing remote name
> > resolving, there's an issue which prevents BlueZ to RNR new devices
> > for 5 minutes after reboot. It's caused by failed_time is init to 0,
> > and is then treated as the timestamp when the device failed RNR.
> > However, actually there is no failure yet.
> >
> > This CL fixes it by always allowing RNR when failed_time = 0.
> >
> > Reviewed-by: Shuo-Peng Liao <deanliao@chromium.org>
> > ---
> >
> > src/device.c | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/device.c b/src/device.c
> > index f2447c478e..16b1ed5bd3 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -4487,10 +4487,15 @@ bool device_is_name_resolve_allowed(struct btd_device *device)
> >
> > clock_gettime(CLOCK_MONOTONIC, &now);
> >
> > - /* If now < failed_time, it means the clock has somehow turned back,
> > - * possibly because of system restart. Allow name request in this case.
> > + /* Allow name request for any of these cases:
> > + * (1) failed_time is empty (0). Meaning no prior failure.
> > + * (2) now < failed_time. Meaning the clock has somehow turned back,
> > + * possibly because of system restart. Allow just to be safe.
> > + * (3) now >= failed_time + name_request_retry_delay. Meaning the
> > + * period of not sending name request is over.
> > */
> > - return now.tv_sec < device->name_resolve_failed_time ||
> > + return device->name_resolve_failed_time == 0 ||
> > + now.tv_sec < device->name_resolve_failed_time ||
> > now.tv_sec >= device->name_resolve_failed_time +
> > btd_opts.name_request_retry_delay;
> > }
>
> I have the feeling this becomes complex and hard to follow. I appreciate the comment above, but just for thoughts, would it be better to do something like this:
>
> /* failed_time is empty (0). Meaning no prior failure. */
> if (device->name_..failed_time == 0)
> return true;
>
> ..
>
> return false;
>
I actually prefer your suggestion, this makes the code much easier to read.
However it's not a very common pattern I observe so I went with the
previous approach.
I have sent a v2 to separate the big if, please take a look.
Thanks,
Archie
> Regards
>
> Marcel
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-13 7:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-13 6:16 [Bluez PATCH] device: Fix device can't be scanned for 5 mins after reboot Archie Pusaka
2022-01-13 7:05 ` Marcel Holtmann
2022-01-13 7:51 ` Archie Pusaka
2022-01-13 7:31 ` [Bluez] " bluez.test.bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.