RE: [PATCH v3 2/2] Bluetooth: hci_event: keep HCI_LE_ADV set if the host cancelled

Oh, you're right. The test is too narrow - a connection timeout on the central path should not clear it either.

The clear exists because advertising stops when a peripheral connection is created, so the role seems more relevant than the status - but ev->role is not dependable on a failed event, and the conn lookup that would give a trustworthy role happens after this point.

We could move the clear below the lookup and gate it on conn->role, or simply !status ?

Valentin Kindschi

-----Message d'origine-----
De : Luiz Augusto von Dentz <[email protected]> 
Envoyé : lundi, 17 août 2026 18:12
À : Valentin Kindschi <[email protected]>
Cc : [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Objet : Re: [PATCH v3 2/2] Bluetooth: hci_event: keep HCI_LE_ADV set if the host cancelled

Hi Valentin,

On Mon, Aug 17, 2026 at 11:03 AM Valentin Kindschi <[email protected]> wrote:
>
> le_conn_complete_evt() clears HCI_LE_ADV before looking at the event 
> status, on the premise stated in its comment that all controllers stop 
> advertising when a connection is created.
>
> That premise fails for Unknown Connection Identifier (0x02), which is 
> what an HCI_LE_Connection_Complete carries after the host issued LE 
> Create Connection Cancel: no connection was created and the controller 
> never stopped advertising. Clearing the flag there makes the host 
> believe advertising is off while the controller has it on.
>
> Other non-zero statuses must keep clearing it. Advertising Timeout
> (0x3c) in particular means the controller gave up advertising on its 
> own, so the flag has to go; leaving it set would make the "already 
> advertising" shortcut in hci_schedule_adv_instance_sync() skip the HCI 
> commands and silently stop advertising altogether.
>
> With legacy advertising the disagreement is self-sustaining. On the 
> next software rotation tick hci_enable_advertising_sync() runs:
>
>   - hci_disable_advertising_sync() returns early without sending
>     anything, because HCI_LE_ADV is clear;
>   - LE Set Advertising Parameters is then sent while the controller is
>     still advertising, and is correctly rejected with Command Disallowed
>     (0x0c);
>   - the function returns before LE Set Advertising Enable, so nothing
>     re-sets HCI_LE_ADV.
>
> hci_schedule_adv_instance_sync() re-arms adv_instance_expire every 
> HCI_DEFAULT_ADV_DURATION (2 s) and its "already advertising" shortcut 
> tests HCI_LE_ADV, which can no longer become true, so the command is 
> retried every 2 s indefinitely:
>
>   Bluetooth: hci0: Opcode 0x2006 failed: -16
>
> Captured on a BCM43455 (no LE Extended Advertising) after a central 
> connection attempt timed out and was cancelled:
>
>   LE Set Advertising Parameters (0x2006)   Success
>   LE Set Advertising Enable     (0x200a)   Success        HCI_LE_ADV set
>   LE Create Connection Cancel   (0x200e)   Success
>   LE Connection Complete                   Unknown Conn Id  <- flag cleared
>   LE Set Advertising Parameters (0x2006)   Command Disallowed  [+2.033 s]
>   LE Set Advertising Parameters (0x2006)   Command Disallowed  [+2.016 s]
>   ...
>
> Keep the flag only for the host-cancelled case.
>
> Fixes: fbd96c151cdc ("Bluetooth: Fix clearing HCI_LE_ADV for LE 
> connections")
> Cc: [email protected]
> Assisted-by: Claude:claude-opus-5 btmon
> Signed-off-by: Valentin Kindschi <[email protected]>
> ---
> Changes in v2:
> - Rebased onto bluetooth-next; no functional change.
>   v1's hci_event.c context lacked the hci_store_wake_reason() call
>   present in mainline, so the hunk did not apply.
>
>  net/bluetooth/hci_event.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -5720,10 +5720,12 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
>         hci_dev_lock(hdev);
>         hci_store_wake_reason(hdev, bdaddr, bdaddr_type);
>
> -       /* All controllers implicitly stop advertising in the event of a
> -        * connection, so ensure that the state bit is cleared.
> +       /* Advertising stops when a connection is created, and when the
> +        * controller gives up advertising on its own. It keeps advertising
> +        * when the host cancelled an outgoing connection.
>          */
> -       hci_dev_clear_flag(hdev, HCI_LE_ADV);
> +       if (status != HCI_ERROR_UNKNOWN_CONN_ID)
> +               hci_dev_clear_flag(hdev, HCI_LE_ADV);

Hmm, I wonder if this is not valid for all status != 0 though, for example if the connection timeout we probably shouldn't clear it either.

>         /* Check for existing connection:
>          *
> --
> 2.34.1



--
Luiz Augusto von Dentz