| From 582e383178d5fe27bfc5f7c076a980ac20482129 Mon Sep 17 00:00:00 2001 |
| From: Archie Pusaka <apusaka@chromium.org> |
| Date: Fri, 13 Aug 2021 18:02:11 +0800 |
| Subject: [PATCH] CHROMIUM: Bluetooth: Fix receiving |
| HCI_LE_Advertising_Set_Terminated event |
| |
| 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 (*). |
| |
| There are a couple of issues with the current situation: |
| (1) RTL8822 sends 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 also temporarily disable advertisements. However, |
| the userspace thinks the advertisements are removed. This causes |
| the advHealth tests involving suspend resume to fail since we |
| are testing to remove the advertisements after resume, but the |
| advertisements are already removed. |
| (2) When connection is successful, the related advertisement is |
| temporarily disabled, as opposed to being removed when other |
| errors arise. Our code mishandled this scenario and removes the |
| advertisement even on successful connection. This causes this |
| advertisement slot goes into a bad state and can't be reused. |
| |
| This patch solves both issues by checking HCI_ERROR_CANCELLED_BY_HOST |
| before everything else, and remove advertisements only on failure |
| case. |
| |
| Additionally, this patch brings the shape of the code nearer to those |
| in upstream. |
| |
| BUG=b:196523329,b:196521730 |
| TEST=AdvHealth pass on Hayato and Sarien, also manually tests issue |
| (1) on Hayato and issue (2) on Sarien. |
| |
| Signed-off-by: Archie Pusaka <apusaka@chromium.org> |
| |
| Change-Id: I0dad14ce0373c4919f5447fc51b68efe570e5f8b |
| Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3112700 |
| Tested-by: Archie Pusaka <apusaka@chromium.org> |
| Reviewed-by: Yun-Hao Chung <howardchung@chromium.org> |
| Commit-Queue: Archie Pusaka <apusaka@chromium.org> |
| --- |
| net/bluetooth/hci_event.c | 39 +++++++++++++++++---------------------- |
| 1 file changed, 17 insertions(+), 22 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 |
| @@ -5689,6 +5689,15 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data, |
| |
| bt_dev_dbg(hdev, "status 0x%2.2x", ev->status); |
| |
| + /* Only RTK chips emit HCI_ERROR_CANCELLED_BY_HOST, which they are not |
| + * supposed to do (consult BT spec vol4 Part E sec 7.7.65.18). This |
| + * event is being fired as a result of a hci_cp_le_set_ext_adv_enable |
| + * disable request, which will have its own callback and cleanup via |
| + * the hci_cc_le_set_ext_adv_enable path. |
| + */ |
| + if (ev->status == HCI_ERROR_CANCELLED_BY_HOST) |
| + return; |
| + |
| adv = hci_find_adv_instance(hdev, ev->handle); |
| |
| /* The Bluetooth Core 5.3 specification clearly states that this event |
| @@ -5711,6 +5720,12 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data, |
| hci_remove_adv_instance(hdev, ev->handle); |
| mgmt_advertising_removed(NULL, hdev, ev->handle); |
| |
| + /* If we are no longer advertising, clear HCI_LE_ADV */ |
| + if (list_empty(&hdev->adv_instances) && |
| + !hdev->ext_directed_advertising) { |
| + hci_dev_clear_flag(hdev, HCI_LE_ADV); |
| + } |
| + |
| list_for_each_entry_safe(adv, n, &hdev->adv_instances, list) { |
| if (adv->enabled) |
| return; |
| @@ -5733,36 +5748,16 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data, |
| |
| if (hdev->adv_addr_type != ADDR_LE_DEV_RANDOM || |
| bacmp(&conn->resp_addr, BDADDR_ANY)) |
| - goto cleanup_instance; |
| + return; |
| |
| if (!ev->handle) { |
| bacpy(&conn->resp_addr, &hdev->random_addr); |
| - goto cleanup_instance; |
| + return; |
| } |
| |
| if (adv) |
| bacpy(&conn->resp_addr, &adv->random_addr); |
| } |
| - |
| -cleanup_instance: |
| - /* Since the controller tells us this instance is no longer active, we |
| - * remove it. |
| - * |
| - * One caveat is that if the status is HCI_ERROR_CANCELLED_BY_HOST, this |
| - * event is being fired as a result of a hci_cp_le_set_ext_adv_enable |
| - * disable request, which will have its own callback and cleanup via |
| - * the hci_cc_le_set_ext_adv_enable path. If this is the case, we will |
| - * not remove the instance, as it may only be a temporary disable. |
| - */ |
| - if (ev->status != HCI_ERROR_CANCELLED_BY_HOST) { |
| - hci_remove_adv_instance(hdev, ev->handle); |
| - |
| - /* If we are no longer advertising, clear HCI_LE_ADV */ |
| - if (list_empty(&hdev->adv_instances) && |
| - !hdev->ext_directed_advertising) { |
| - hci_dev_clear_flag(hdev, HCI_LE_ADV); |
| - } |
| - } |
| } |
| |
| static void hci_le_conn_update_complete_evt(struct hci_dev *hdev, void *data, |
| -- |
| 2.35.0.rc0.227.g00780c9af4-goog |
| |