* [PATCH v2] adapter: Cancel the service authorization when remote is disconnected @ 2024-08-26 9:00 Cheng Jiang 2024-08-26 10:43 ` [v2] " bluez.test.bot 2024-08-26 13:43 ` [PATCH v2] " Luiz Augusto von Dentz 0 siblings, 2 replies; 6+ messages in thread From: Cheng Jiang @ 2024-08-26 9:00 UTC (permalink / raw) To: linux-bluetooth If the remote device drops the connection before DUT confirm the service authorization, the DUT still must wait for service authorization timeout before processing future request. Cancel the service authorization request when connection is dropped. --- src/adapter.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/adapter.c b/src/adapter.c index 245de4456..3ad000425 100644 --- a/src/adapter.c +++ b/src/adapter.c @@ -8502,6 +8502,25 @@ static void disconnect_notify(struct btd_device *dev, uint8_t reason) } } +static void adapter_cancel_service_auth(struct btd_adapter *adapter, + struct btd_device *device) +{ + struct service_auth *auth; + GList *l; + + auth = NULL; + for (l = adapter->auths->head; l != NULL; l = l->next) { + auth = l->data; + if (auth->device == device) + break; + } + if (auth != NULL) { + DBG("Cancel pending auth: %s", auth->uuid); + g_queue_remove(auth->adapter->auths, auth); + service_auth_cancel(auth); + } +} + static void dev_disconnected(struct btd_adapter *adapter, const struct mgmt_addr_info *addr, uint8_t reason) @@ -8516,6 +8535,7 @@ static void dev_disconnected(struct btd_adapter *adapter, device = btd_adapter_find_device(adapter, &addr->bdaddr, addr->type); if (device) { adapter_remove_connection(adapter, device, addr->type); + adapter_cancel_service_auth(adapter, device); disconnect_notify(device, reason); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [v2] adapter: Cancel the service authorization when remote is disconnected 2024-08-26 9:00 [PATCH v2] adapter: Cancel the service authorization when remote is disconnected Cheng Jiang @ 2024-08-26 10:43 ` bluez.test.bot 2024-08-26 13:43 ` [PATCH v2] " Luiz Augusto von Dentz 1 sibling, 0 replies; 6+ messages in thread From: bluez.test.bot @ 2024-08-26 10:43 UTC (permalink / raw) To: linux-bluetooth, quic_chejiang [-- Attachment #1: Type: text/plain, Size: 949 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=883234 ---Test result--- Test Summary: CheckPatch PASS 0.41 seconds GitLint PASS 0.27 seconds BuildEll PASS 24.55 seconds BluezMake PASS 1728.48 seconds MakeCheck PASS 13.32 seconds MakeDistcheck PASS 182.69 seconds CheckValgrind PASS 254.20 seconds CheckSmatch PASS 355.43 seconds bluezmakeextell PASS 119.41 seconds IncrementalBuild PASS 1507.74 seconds ScanBuild PASS 1003.07 seconds --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] adapter: Cancel the service authorization when remote is disconnected 2024-08-26 9:00 [PATCH v2] adapter: Cancel the service authorization when remote is disconnected Cheng Jiang 2024-08-26 10:43 ` [v2] " bluez.test.bot @ 2024-08-26 13:43 ` Luiz Augusto von Dentz 2024-08-26 14:30 ` Cheng Jiang 1 sibling, 1 reply; 6+ messages in thread From: Luiz Augusto von Dentz @ 2024-08-26 13:43 UTC (permalink / raw) To: Cheng Jiang; +Cc: linux-bluetooth Hi Cheng, On Mon, Aug 26, 2024 at 5:01 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote: > > If the remote device drops the connection before DUT confirm the > service authorization, the DUT still must wait for service > authorization timeout before processing future request. > > Cancel the service authorization request when connection is dropped. > --- > src/adapter.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/src/adapter.c b/src/adapter.c > index 245de4456..3ad000425 100644 > --- a/src/adapter.c > +++ b/src/adapter.c > @@ -8502,6 +8502,25 @@ static void disconnect_notify(struct btd_device *dev, uint8_t reason) > } > } > > +static void adapter_cancel_service_auth(struct btd_adapter *adapter, > + struct btd_device *device) > +{ > + struct service_auth *auth; > + GList *l; > + > + auth = NULL; > + for (l = adapter->auths->head; l != NULL; l = l->next) { > + auth = l->data; > + if (auth->device == device) > + break; > + } > + if (auth != NULL) { > + DBG("Cancel pending auth: %s", auth->uuid); > + g_queue_remove(auth->adapter->auths, auth); > + service_auth_cancel(auth); > + } > +} > + > static void dev_disconnected(struct btd_adapter *adapter, > const struct mgmt_addr_info *addr, > uint8_t reason) > @@ -8516,6 +8535,7 @@ static void dev_disconnected(struct btd_adapter *adapter, > device = btd_adapter_find_device(adapter, &addr->bdaddr, addr->type); > if (device) { > adapter_remove_connection(adapter, device, addr->type); > + adapter_cancel_service_auth(adapter, device); This shall probably be placed together with device_cancel_authentication in adapter_remove_connection but we need to track in what bearer the authorization is for and remove all authorization requests like in btd_adapter_remove_device: l = adapter->auths->head; while (l != NULL) { struct service_auth *auth = l->data; GList *next = g_list_next(l); if (auth->device != dev) { l = next; continue; } g_queue_delete_link(adapter->auths, l); l = next; service_auth_cancel(auth); } I'd probably move the above code to a helper function so it can be called from different places, as for doing this on a per bearer basis would probably need to add bearer information to btd_service but I guess that can be treated separately. > disconnect_notify(device, reason); > } > > -- > 2.25.1 > > -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] adapter: Cancel the service authorization when remote is disconnected 2024-08-26 13:43 ` [PATCH v2] " Luiz Augusto von Dentz @ 2024-08-26 14:30 ` Cheng Jiang 2024-09-19 19:53 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 6+ messages in thread From: Cheng Jiang @ 2024-08-26 14:30 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth Hi Luiz, Thank you for your feedback. Please check the comment inline. On 8/26/2024 9:43 PM, Luiz Augusto von Dentz wrote: > Hi Cheng, > > On Mon, Aug 26, 2024 at 5:01 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote: >> >> If the remote device drops the connection before DUT confirm the >> service authorization, the DUT still must wait for service >> authorization timeout before processing future request. >> >> Cancel the service authorization request when connection is dropped. >> --- >> src/adapter.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/src/adapter.c b/src/adapter.c >> index 245de4456..3ad000425 100644 >> --- a/src/adapter.c >> +++ b/src/adapter.c >> @@ -8502,6 +8502,25 @@ static void disconnect_notify(struct btd_device *dev, uint8_t reason) >> } >> } >> >> +static void adapter_cancel_service_auth(struct btd_adapter *adapter, >> + struct btd_device *device) >> +{ >> + struct service_auth *auth; >> + GList *l; >> + >> + auth = NULL; >> + for (l = adapter->auths->head; l != NULL; l = l->next) { >> + auth = l->data; >> + if (auth->device == device) >> + break; >> + } >> + if (auth != NULL) { >> + DBG("Cancel pending auth: %s", auth->uuid); >> + g_queue_remove(auth->adapter->auths, auth); >> + service_auth_cancel(auth); >> + } >> +} >> + >> static void dev_disconnected(struct btd_adapter *adapter, >> const struct mgmt_addr_info *addr, >> uint8_t reason) >> @@ -8516,6 +8535,7 @@ static void dev_disconnected(struct btd_adapter *adapter, >> device = btd_adapter_find_device(adapter, &addr->bdaddr, addr->type); >> if (device) { >> adapter_remove_connection(adapter, device, addr->type); >> + adapter_cancel_service_auth(adapter, device); > > This shall probably be placed together with > device_cancel_authentication in adapter_remove_connection but we need > to track in what bearer the authorization is for and remove all > authorization requests like in btd_adapter_remove_device: Yes. It can be put in device_cancel_authentication, but the condition to call this function in adapter_remove_connection need change. device_is_authenticating is only true when request, notify or confirm pincode/passkey. As for bearer, the service authorize is only happened on BDADDR_BREDR. So it can be called when the address type is BDADDR_BREDR in dev_disconnected. > > l = adapter->auths->head; > while (l != NULL) { > struct service_auth *auth = l->data; > GList *next = g_list_next(l); > > if (auth->device != dev) { > l = next; > continue; > } > > g_queue_delete_link(adapter->auths, l); > l = next; > > service_auth_cancel(auth); > } > > I'd probably move the above code to a helper function so it can be > called from different places, as for doing this on a per bearer basis > would probably need to add bearer information to btd_service but I > guess that can be treated separately. Yes, it's great. Need use the above code to cancel all pending authorize. > > >> disconnect_notify(device, reason); >> } >> >> -- >> 2.25.1 >> >> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] adapter: Cancel the service authorization when remote is disconnected 2024-08-26 14:30 ` Cheng Jiang @ 2024-09-19 19:53 ` Luiz Augusto von Dentz 2024-09-20 1:56 ` Cheng Jiang 0 siblings, 1 reply; 6+ messages in thread From: Luiz Augusto von Dentz @ 2024-09-19 19:53 UTC (permalink / raw) To: Cheng Jiang; +Cc: linux-bluetooth Hi Cheng, On Mon, Aug 26, 2024 at 10:30 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote: > > Hi Luiz, > > Thank you for your feedback. Please check the comment inline. > > On 8/26/2024 9:43 PM, Luiz Augusto von Dentz wrote: > > Hi Cheng, > > > > On Mon, Aug 26, 2024 at 5:01 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote: > >> > >> If the remote device drops the connection before DUT confirm the > >> service authorization, the DUT still must wait for service > >> authorization timeout before processing future request. > >> > >> Cancel the service authorization request when connection is dropped. > >> --- > >> src/adapter.c | 20 ++++++++++++++++++++ > >> 1 file changed, 20 insertions(+) > >> > >> diff --git a/src/adapter.c b/src/adapter.c > >> index 245de4456..3ad000425 100644 > >> --- a/src/adapter.c > >> +++ b/src/adapter.c > >> @@ -8502,6 +8502,25 @@ static void disconnect_notify(struct btd_device *dev, uint8_t reason) > >> } > >> } > >> > >> +static void adapter_cancel_service_auth(struct btd_adapter *adapter, > >> + struct btd_device *device) > >> +{ > >> + struct service_auth *auth; > >> + GList *l; > >> + > >> + auth = NULL; > >> + for (l = adapter->auths->head; l != NULL; l = l->next) { > >> + auth = l->data; > >> + if (auth->device == device) > >> + break; > >> + } > >> + if (auth != NULL) { > >> + DBG("Cancel pending auth: %s", auth->uuid); > >> + g_queue_remove(auth->adapter->auths, auth); > >> + service_auth_cancel(auth); > >> + } > >> +} > >> + > >> static void dev_disconnected(struct btd_adapter *adapter, > >> const struct mgmt_addr_info *addr, > >> uint8_t reason) > >> @@ -8516,6 +8535,7 @@ static void dev_disconnected(struct btd_adapter *adapter, > >> device = btd_adapter_find_device(adapter, &addr->bdaddr, addr->type); > >> if (device) { > >> adapter_remove_connection(adapter, device, addr->type); > >> + adapter_cancel_service_auth(adapter, device); > > > > This shall probably be placed together with > > device_cancel_authentication in adapter_remove_connection but we need > > to track in what bearer the authorization is for and remove all > > authorization requests like in btd_adapter_remove_device: > Yes. It can be put in device_cancel_authentication, but the condition to call > this function in adapter_remove_connection need change. device_is_authenticating > is only true when request, notify or confirm pincode/passkey. > As for bearer, the service authorize is only happened on BDADDR_BREDR. So it > can be called when the address type is BDADDR_BREDR in dev_disconnected. > > > > l = adapter->auths->head; > > while (l != NULL) { > > struct service_auth *auth = l->data; > > GList *next = g_list_next(l); > > > > if (auth->device != dev) { > > l = next; > > continue; > > } > > > > g_queue_delete_link(adapter->auths, l); > > l = next; > > > > service_auth_cancel(auth); > > } > > > > I'd probably move the above code to a helper function so it can be > > called from different places, as for doing this on a per bearer basis > > would probably need to add bearer information to btd_service but I > > guess that can be treated separately. > Yes, it's great. Need use the above code to cancel all pending authorize. Looks like you never send another version of this one, are you still planning on having this fixed? > > > > > >> disconnect_notify(device, reason); > >> } > >> > >> -- > >> 2.25.1 > >> > >> > > > > > -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] adapter: Cancel the service authorization when remote is disconnected 2024-09-19 19:53 ` Luiz Augusto von Dentz @ 2024-09-20 1:56 ` Cheng Jiang 0 siblings, 0 replies; 6+ messages in thread From: Cheng Jiang @ 2024-09-20 1:56 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth Hi Luiz, Sorry for the delay, will send the new patch soon. On 9/20/2024 3:53 AM, Luiz Augusto von Dentz wrote: > Hi Cheng, > > On Mon, Aug 26, 2024 at 10:30 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote: >> >> Hi Luiz, >> >> Thank you for your feedback. Please check the comment inline. >> >> On 8/26/2024 9:43 PM, Luiz Augusto von Dentz wrote: >>> Hi Cheng, >>> >>> On Mon, Aug 26, 2024 at 5:01 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote: >>>> >>>> If the remote device drops the connection before DUT confirm the >>>> service authorization, the DUT still must wait for service >>>> authorization timeout before processing future request. >>>> >>>> Cancel the service authorization request when connection is dropped. >>>> --- >>>> src/adapter.c | 20 ++++++++++++++++++++ >>>> 1 file changed, 20 insertions(+) >>>> >>>> diff --git a/src/adapter.c b/src/adapter.c >>>> index 245de4456..3ad000425 100644 >>>> --- a/src/adapter.c >>>> +++ b/src/adapter.c >>>> @@ -8502,6 +8502,25 @@ static void disconnect_notify(struct btd_device *dev, uint8_t reason) >>>> } >>>> } >>>> >>>> +static void adapter_cancel_service_auth(struct btd_adapter *adapter, >>>> + struct btd_device *device) >>>> +{ >>>> + struct service_auth *auth; >>>> + GList *l; >>>> + >>>> + auth = NULL; >>>> + for (l = adapter->auths->head; l != NULL; l = l->next) { >>>> + auth = l->data; >>>> + if (auth->device == device) >>>> + break; >>>> + } >>>> + if (auth != NULL) { >>>> + DBG("Cancel pending auth: %s", auth->uuid); >>>> + g_queue_remove(auth->adapter->auths, auth); >>>> + service_auth_cancel(auth); >>>> + } >>>> +} >>>> + >>>> static void dev_disconnected(struct btd_adapter *adapter, >>>> const struct mgmt_addr_info *addr, >>>> uint8_t reason) >>>> @@ -8516,6 +8535,7 @@ static void dev_disconnected(struct btd_adapter *adapter, >>>> device = btd_adapter_find_device(adapter, &addr->bdaddr, addr->type); >>>> if (device) { >>>> adapter_remove_connection(adapter, device, addr->type); >>>> + adapter_cancel_service_auth(adapter, device); >>> >>> This shall probably be placed together with >>> device_cancel_authentication in adapter_remove_connection but we need >>> to track in what bearer the authorization is for and remove all >>> authorization requests like in btd_adapter_remove_device: >> Yes. It can be put in device_cancel_authentication, but the condition to call >> this function in adapter_remove_connection need change. device_is_authenticating >> is only true when request, notify or confirm pincode/passkey. >> As for bearer, the service authorize is only happened on BDADDR_BREDR. So it >> can be called when the address type is BDADDR_BREDR in dev_disconnected. >>> >>> l = adapter->auths->head; >>> while (l != NULL) { >>> struct service_auth *auth = l->data; >>> GList *next = g_list_next(l); >>> >>> if (auth->device != dev) { >>> l = next; >>> continue; >>> } >>> >>> g_queue_delete_link(adapter->auths, l); >>> l = next; >>> >>> service_auth_cancel(auth); >>> } >>> >>> I'd probably move the above code to a helper function so it can be >>> called from different places, as for doing this on a per bearer basis >>> would probably need to add bearer information to btd_service but I >>> guess that can be treated separately. >> Yes, it's great. Need use the above code to cancel all pending authorize. > > Looks like you never send another version of this one, are you still > planning on having this fixed? > >>> >>> >>>> disconnect_notify(device, reason); >>>> } >>>> >>>> -- >>>> 2.25.1 >>>> >>>> >>> >>> >> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-20 1:56 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-26 9:00 [PATCH v2] adapter: Cancel the service authorization when remote is disconnected Cheng Jiang 2024-08-26 10:43 ` [v2] " bluez.test.bot 2024-08-26 13:43 ` [PATCH v2] " Luiz Augusto von Dentz 2024-08-26 14:30 ` Cheng Jiang 2024-09-19 19:53 ` Luiz Augusto von Dentz 2024-09-20 1:56 ` Cheng Jiang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox