| Message ID | 20211103133225.v2.1.I3ba1a76d72da5a813cf6e6f219838c9ef28c5eaa@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] Bluetooth: Ignore HCI_ERROR_CANCELLED_BY_HOST on adv set terminated event | expand |
Hi Maintainers, Could you take a look at this patch? Thanks, Archie On Wed, 3 Nov 2021 at 13:33, Archie Pusaka <apusaka@google.com> wrote: > > From: Archie Pusaka <apusaka@chromium.org> > > This event is received when the controller stops advertising, > specifically for these three reasons: > (a) Connection is successfully created (success). > (b) Timeout is reached (error). > (c) Number of advertising events is reached (error). > (*) This event is NOT generated when the host stops the advertisement. > Refer to the BT spec ver 5.3 vol 4 part E sec 7.7.65.18. Note that the > section was revised from BT spec ver 5.0 vol 2 part E sec 7.7.65.18 > which was ambiguous about (*). > > Some chips (e.g. RTL8822CE) send this event when the host stops the > advertisement with status = HCI_ERROR_CANCELLED_BY_HOST (due to (*) > above). This is treated as an error and the advertisement will be > removed and userspace will be informed via MGMT event. > > On suspend, we are supposed to temporarily disable advertisements, > and continue advertising on resume. However, due to the behavior > above, the advertisements are removed instead. > > This patch returns early if HCI_ERROR_CANCELLED_BY_HOST is received. > > Btmon snippet of the unexpected behavior: > @ MGMT Command: Remove Advertising (0x003f) plen 1 > Instance: 1 > < HCI Command: LE Set Extended Advertising Enable (0x08|0x0039) plen 6 > Extended advertising: Disabled (0x00) > Number of sets: 1 (0x01) > Entry 0 > Handle: 0x01 > Duration: 0 ms (0x00) > Max ext adv events: 0 > > HCI Event: LE Meta Event (0x3e) plen 6 > LE Advertising Set Terminated (0x12) > Status: Operation Cancelled by Host (0x44) > Handle: 1 > Connection handle: 0 > Number of completed extended advertising events: 5 > > HCI Event: Command Complete (0x0e) plen 4 > LE Set Extended Advertising Enable (0x08|0x0039) ncmd 2 > Status: Success (0x00) > > Signed-off-by: Archie Pusaka <apusaka@chromium.org> > > --- > > Changes in v2: > * Split clearing HCI_LE_ADV into its own patch > * Reword comments > > include/net/bluetooth/hci.h | 1 + > net/bluetooth/hci_event.c | 12 ++++++++++++ > 2 files changed, 13 insertions(+) > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > index 63065bc01b76..84db6b275231 100644 > --- a/include/net/bluetooth/hci.h > +++ b/include/net/bluetooth/hci.h > @@ -566,6 +566,7 @@ enum { > #define HCI_ERROR_INVALID_LL_PARAMS 0x1e > #define HCI_ERROR_UNSPECIFIED 0x1f > #define HCI_ERROR_ADVERTISING_TIMEOUT 0x3c > +#define HCI_ERROR_CANCELLED_BY_HOST 0x44 > > /* Flow control modes */ > #define HCI_FLOW_CTL_MODE_PACKET_BASED 0x00 > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index d4b75a6cfeee..7d875927c48b 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -5538,6 +5538,18 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, struct sk_buff *skb) > > adv = hci_find_adv_instance(hdev, ev->handle); > > + /* The Bluetooth Core 5.3 specification clearly states that this event > + * shall not be sent when the Host disables the advertising set. So in > + * case of HCI_ERROR_CANCELLED_BY_HOST, just ignore the event. > + * > + * When the Host disables an advertising set, all cleanup is done via > + * its command callback and not needed to be duplicated here. > + */ > + if (ev->status == HCI_ERROR_CANCELLED_BY_HOST) { > + bt_dev_warn_ratelimited(hdev, "Unexpected advertising set terminated event"); > + return; > + } > + > if (ev->status) { > if (!adv) > return; > -- > 2.33.1.1089.g2158813163f-goog >
Hi Archie, > Could you take a look at this patch? > patch is fine, I am just waiting for the resolution of our pull request before applying new patches. Regards Marcel
Hi Marcel, Thanks for your prompt reply. On Wed, 10 Nov 2021 at 17:48, Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Archie, > > > Could you take a look at this patch? > > > > patch is fine, I am just waiting for the resolution of our pull request before applying new patches. > > Regards > > Marcel >
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index 63065bc01b76..84db6b275231 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -566,6 +566,7 @@ enum { #define HCI_ERROR_INVALID_LL_PARAMS 0x1e #define HCI_ERROR_UNSPECIFIED 0x1f #define HCI_ERROR_ADVERTISING_TIMEOUT 0x3c +#define HCI_ERROR_CANCELLED_BY_HOST 0x44 /* Flow control modes */ #define HCI_FLOW_CTL_MODE_PACKET_BASED 0x00 diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index d4b75a6cfeee..7d875927c48b 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -5538,6 +5538,18 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, struct sk_buff *skb) adv = hci_find_adv_instance(hdev, ev->handle); + /* The Bluetooth Core 5.3 specification clearly states that this event + * shall not be sent when the Host disables the advertising set. So in + * case of HCI_ERROR_CANCELLED_BY_HOST, just ignore the event. + * + * When the Host disables an advertising set, all cleanup is done via + * its command callback and not needed to be duplicated here. + */ + if (ev->status == HCI_ERROR_CANCELLED_BY_HOST) { + bt_dev_warn_ratelimited(hdev, "Unexpected advertising set terminated event"); + return; + } + if (ev->status) { if (!adv) return;