Linux bluetooth development
 help / color / mirror / Atom feed
From: Valentin Kindschi <valentin.kindschi@fiveco.ch>
To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: "linux-bluetooth@vger.kernel.org"
	<linux-bluetooth@vger.kernel.org>,
	"marcel@holtmann.org" <marcel@holtmann.org>,
	"johan.hedberg@gmail.com" <johan.hedberg@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: RE: [PATCH v3 2/2] Bluetooth: hci_event: keep HCI_LE_ADV set if the host cancelled
Date: Tue, 18 Aug 2026 13:30:09 +0000	[thread overview]
Message-ID: <a5c42f27a38a490683b730694f7b241c@fiveco.ch> (raw)
In-Reply-To: <CABBYNZKLcM2L9VTNhyr=O+TMHK-CoyspCQA+yU_hP2ER-SOS+g@mail.gmail.com>

Done, v4 sent using !status : a connection timeout leaves the controller advertising just the same, so no reason to single out Unknown Conn Id. Tested on the device: two cancelled connections, no Command Disallowed after, advertising still up.

On extended advertising, I think it argues for the change: hci_cc_le_set_ext_adv_enable() already checks hdev->adv_instances before clearing HCI_LE_ADV, and the unconditional clear here discards that. !status at least stops failed attempts from corrupting it.

Doing it properly for multiple sets looks like a separate patch. My hardware has no ext adv, so that part is code reading, not tested.

Valentin 

-----Message d'origine-----
De : Luiz Augusto von Dentz <luiz.dentz@gmail.com> 
Envoyé : lundi, 17 août 2026 18:43
À : Valentin Kindschi <valentin.kindschi@fiveco.ch>
Cc : linux-bluetooth@vger.kernel.org; marcel@holtmann.org; johan.hedberg@gmail.com; linux-kernel@vger.kernel.org; stable@vger.kernel.org
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 12:31 PM Valentin Kindschi <valentin.kindschi@fiveco.ch> wrote:
>
> 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 ?

Try with !status, we need to confirm that doesn't break any CI test either. That said, it does look like it would clear it also for extended advertising where there could be multiple instances advertising, so I'm not sure if it's correct to assume that if one instance connects all others shall also be considered connected.

> Valentin Kindschi
>
> -----Message d'origine-----
> De : Luiz Augusto von Dentz <luiz.dentz@gmail.com> Envoyé : lundi, 17 
> août 2026 18:12 À : Valentin Kindschi <valentin.kindschi@fiveco.ch> Cc 
> : linux-bluetooth@vger.kernel.org; marcel@holtmann.org; 
> johan.hedberg@gmail.com; linux-kernel@vger.kernel.org; 
> stable@vger.kernel.org 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 <valentin.kindschi@fiveco.ch> 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: stable@vger.kernel.org
> > Assisted-by: Claude:claude-opus-5 btmon
> > Signed-off-by: Valentin Kindschi <valentin.kindschi@fiveco.ch>
> > ---
> > 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



--
Luiz Augusto von Dentz

      reply	other threads:[~2026-08-18 13:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17 15:02 [PATCH v3 0/2] Bluetooth: fix endless adv params retry after a cancelled connection Valentin Kindschi
2026-08-17 15:02 ` [PATCH v3 1/2] Bluetooth: hci_conn: re-enable advertising only for peripheral role Valentin Kindschi
2026-08-17 15:49   ` Bluetooth: fix endless adv params retry after a cancelled connection bluez.test.bot
2026-08-17 15:02 ` [PATCH v3 2/2] Bluetooth: hci_event: keep HCI_LE_ADV set if the host cancelled Valentin Kindschi
2026-08-17 16:11   ` Luiz Augusto von Dentz
2026-08-17 16:31     ` Valentin Kindschi
2026-08-17 16:42       ` Luiz Augusto von Dentz
2026-08-18 13:30         ` Valentin Kindschi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a5c42f27a38a490683b730694f7b241c@fiveco.ch \
    --to=valentin.kindschi@fiveco.ch \
    --cc=johan.hedberg@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox