From patchwork Tue May 24 20:14:24 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Manish Mandlik X-Patchwork-Id: 575803 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C096FC433FE for ; Tue, 24 May 2022 20:14:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241433AbiEXUOb (ORCPT ); Tue, 24 May 2022 16:14:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47562 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241303AbiEXUOa (ORCPT ); Tue, 24 May 2022 16:14:30 -0400 Received: from mail-yw1-x1149.google.com (mail-yw1-x1149.google.com [IPv6:2607:f8b0:4864:20::1149]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A84A882171 for ; Tue, 24 May 2022 13:14:28 -0700 (PDT) Received: by mail-yw1-x1149.google.com with SMTP id 00721157ae682-2dc7bdd666fso161334107b3.7 for ; Tue, 24 May 2022 13:14:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:message-id:mime-version:subject:from:to:cc; bh=2AP4PrI/s0YHrWhwLpknnSCGb+hNIfeHpyhnelir6LA=; b=lKuJwmeRLv6QcUrdPUIMvjhDEd2uOflAMJYwrvodbe5+U4ZLRjIMucfTiy3DMoC4JP J4r2z/1Dc4y9f8xbvavC7yXBViY48UPOfcrSi17PM40BAmK/iyIVvWwtQICkw0Fdwf5u y0c/wiSDQmswOSfi+oKZ2xTEjw89Inv6nVMkqOUjK2aiKRxgysjEG+Brhp8LaiZVMcXd Fz/jM4Ui8xIMrgwM4LbEXwsDuxuCScb52AF7FYAenQ5TmceLmGWKSuSGhjQ7GlXrz69P NuvatMHJxq03vq7pKJYzhonKN3XPohjH7WOKzzlfW6/MJeO6jI6eiPxtZmPrfLTgli1V rvkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:message-id:mime-version:subject:from:to:cc; bh=2AP4PrI/s0YHrWhwLpknnSCGb+hNIfeHpyhnelir6LA=; b=5AGkd1SQ7gUjf5eT9ELMDzAD3SYoUatBRAub+WMNxrotAtqkF/BPD+hGpA9tParzFT WZW0ROJBwPoerZQT/NiC2JYeFrs1frCjnUr3hAumVPr9qG1y2UP4/VS8q9UtELZSVbsl RLz8kjWX+kz8Xz6j99pM9bRwf6xJ3/4D5KkHSqgdwaIAgqS5I6v8yZKtlGfZpPNfwmpQ 2COCF51R+zeWD6EEL6Fq+OJsHuvlPkOViPt/wfAOxttu+QuIIYFQYgi3pVbP5UUKfQEW C17l+nl1KxAp1Jy7kYuREKLyxN5BSPNLpAIpLftjIbckCMAF/c80hNsPKhyZ5PZlr+B7 eEcg== X-Gm-Message-State: AOAM5302+Hgb3gvTUQKzIEDwo6DpI5fwj5li0vcWTHOzqEMTIJEg6Y6x WNps58FXQUireMygTEdZjNykDv9iy2kYbQ== X-Google-Smtp-Source: ABdhPJxbfe44DbIU7wOAlZ4x49WiUw3/hqliKt10ztcZP6peTLJAtaD2y2zswwWMys4Ec1BfZ6DiWDe5PQTxhA== X-Received: from mmandlik.mtv.corp.google.com ([2620:15c:202:201:867d:d253:fd17:d811]) (user=mmandlik job=sendgmr) by 2002:a0d:c901:0:b0:300:132e:b5a9 with SMTP id l1-20020a0dc901000000b00300132eb5a9mr8248620ywd.66.1653423267894; Tue, 24 May 2022 13:14:27 -0700 (PDT) Date: Tue, 24 May 2022 13:14:24 -0700 Message-Id: <20220524131406.1.If745ed1d05d98c002fc84ba60cef99eb786b7caa@changeid> Mime-Version: 1.0 X-Mailer: git-send-email 2.36.1.124.g0e6072fb45-goog Subject: [PATCH 1/2] Bluetooth: hci_sync: Refactor add Adv Monitor From: Manish Mandlik To: marcel@holtmann.org, luiz.dentz@gmail.com Cc: chromeos-bluetooth-upstreaming@chromium.org, linux-bluetooth@vger.kernel.org, Manish Mandlik , Miao-chen Chou , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Johan Hedberg , Paolo Abeni , linux-kernel@vger.kernel.org, netdev@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Make use of hci_cmd_sync_queue for adding an advertisement monitor. Signed-off-by: Manish Mandlik Reviewed-by: Miao-chen Chou --- include/net/bluetooth/hci_core.h | 5 +- net/bluetooth/hci_core.c | 47 ++++----- net/bluetooth/mgmt.c | 121 +++++++---------------- net/bluetooth/msft.c | 161 ++++++++----------------------- 4 files changed, 98 insertions(+), 236 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 5a52a2018b56..59953a7a6328 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1410,10 +1410,8 @@ bool hci_adv_instance_is_scannable(struct hci_dev *hdev, u8 instance); void hci_adv_monitors_clear(struct hci_dev *hdev); void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor); -int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status); int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status); -bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, - int *err); +int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor); bool hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle, int *err); bool hci_remove_all_adv_monitor(struct hci_dev *hdev, int *err); bool hci_is_adv_monitoring(struct hci_dev *hdev); @@ -1875,7 +1873,6 @@ void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev, u8 instance); void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle); int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip); -int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status); int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status); void mgmt_adv_monitor_device_lost(struct hci_dev *hdev, u16 handle, bdaddr_t *bdaddr, u8 addr_type); diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 5abb2ca5b129..bbbbe3203130 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -1873,11 +1873,6 @@ void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor) kfree(monitor); } -int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status) -{ - return mgmt_add_adv_patterns_monitor_complete(hdev, status); -} - int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status) { return mgmt_remove_adv_monitor_complete(hdev, status); @@ -1885,49 +1880,49 @@ int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status) /* Assigns handle to a monitor, and if offloading is supported and power is on, * also attempts to forward the request to the controller. - * Returns true if request is forwarded (result is pending), false otherwise. - * This function requires the caller holds hdev->lock. */ -bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, - int *err) +int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor) { int min, max, handle; + int status = 0; - *err = 0; + if (!monitor) + return -EINVAL; - if (!monitor) { - *err = -EINVAL; - return false; - } + hci_dev_lock(hdev); min = HCI_MIN_ADV_MONITOR_HANDLE; max = HCI_MIN_ADV_MONITOR_HANDLE + HCI_MAX_ADV_MONITOR_NUM_HANDLES; handle = idr_alloc(&hdev->adv_monitors_idr, monitor, min, max, GFP_KERNEL); - if (handle < 0) { - *err = handle; - return false; - } + + hci_dev_unlock(hdev); + + if (handle < 0) + return handle; monitor->handle = handle; if (!hdev_is_powered(hdev)) - return false; + return status; switch (hci_get_adv_monitor_offload_ext(hdev)) { case HCI_ADV_MONITOR_EXT_NONE: - hci_update_passive_scan(hdev); - bt_dev_dbg(hdev, "%s add monitor status %d", hdev->name, *err); + bt_dev_dbg(hdev, "%s add monitor %d status %d", hdev->name, + monitor->handle, status); /* Message was not forwarded to controller - not an error */ - return false; + break; + case HCI_ADV_MONITOR_EXT_MSFT: - *err = msft_add_monitor_pattern(hdev, monitor); - bt_dev_dbg(hdev, "%s add monitor msft status %d", hdev->name, - *err); + hci_req_sync_lock(hdev); + status = msft_add_monitor_pattern(hdev, monitor); + hci_req_sync_unlock(hdev); + bt_dev_dbg(hdev, "%s add monitor %d msft status %d", hdev->name, + monitor->handle, status); break; } - return (*err == 0); + return status; } /* Attempts to tell the controller and free the monitor. If somehow the diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 74937a834648..d04f90698a87 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -4653,75 +4653,21 @@ static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev, return err; } -int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status) -{ - struct mgmt_rp_add_adv_patterns_monitor rp; - struct mgmt_pending_cmd *cmd; - struct adv_monitor *monitor; - int err = 0; - - hci_dev_lock(hdev); - - cmd = pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI, hdev); - if (!cmd) { - cmd = pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR, hdev); - if (!cmd) - goto done; - } - - monitor = cmd->user_data; - rp.monitor_handle = cpu_to_le16(monitor->handle); - - if (!status) { - mgmt_adv_monitor_added(cmd->sk, hdev, monitor->handle); - hdev->adv_monitors_cnt++; - if (monitor->state == ADV_MONITOR_STATE_NOT_REGISTERED) - monitor->state = ADV_MONITOR_STATE_REGISTERED; - hci_update_passive_scan(hdev); - } - - err = mgmt_cmd_complete(cmd->sk, cmd->index, cmd->opcode, - mgmt_status(status), &rp, sizeof(rp)); - mgmt_pending_remove(cmd); - -done: - hci_dev_unlock(hdev); - bt_dev_dbg(hdev, "add monitor %d complete, status %u", - rp.monitor_handle, status); - - return err; -} - static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev, - struct adv_monitor *m, u8 status, - void *data, u16 len, u16 op) + struct adv_monitor *m, void *data, + u16 len, u16 op) { struct mgmt_rp_add_adv_patterns_monitor rp; - struct mgmt_pending_cmd *cmd; + u8 status = MGMT_STATUS_SUCCESS; int err; - bool pending; - - hci_dev_lock(hdev); - - if (status) - goto unlock; if (pending_find(MGMT_OP_SET_LE, hdev) || - pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR, hdev) || - pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI, hdev) || pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev)) { status = MGMT_STATUS_BUSY; - goto unlock; - } - - cmd = mgmt_pending_add(sk, op, hdev, data, len); - if (!cmd) { - status = MGMT_STATUS_NO_RESOURCES; - goto unlock; + goto done; } - cmd->user_data = m; - pending = hci_add_adv_monitor(hdev, m, &err); + err = hci_add_adv_monitor(hdev, m); if (err) { if (err == -ENOSPC || err == -ENOMEM) status = MGMT_STATUS_NO_RESOURCES; @@ -4730,30 +4676,29 @@ static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev, else status = MGMT_STATUS_FAILED; - mgmt_pending_remove(cmd); - goto unlock; + goto done; } - if (!pending) { - mgmt_pending_remove(cmd); - rp.monitor_handle = cpu_to_le16(m->handle); - mgmt_adv_monitor_added(sk, hdev, m->handle); - m->state = ADV_MONITOR_STATE_REGISTERED; - hdev->adv_monitors_cnt++; + hci_dev_lock(hdev); - hci_dev_unlock(hdev); - return mgmt_cmd_complete(sk, hdev->id, op, MGMT_STATUS_SUCCESS, - &rp, sizeof(rp)); - } + rp.monitor_handle = cpu_to_le16(m->handle); + mgmt_adv_monitor_added(sk, hdev, m->handle); + if (m->state == ADV_MONITOR_STATE_NOT_REGISTERED) + m->state = ADV_MONITOR_STATE_REGISTERED; + hdev->adv_monitors_cnt++; + hci_update_passive_scan(hdev); hci_dev_unlock(hdev); - return 0; +done: + bt_dev_dbg(hdev, "add monitor %d complete, status %u", m->handle, + status); -unlock: - hci_free_adv_monitor(hdev, m); - hci_dev_unlock(hdev); - return mgmt_cmd_status(sk, hdev->id, op, status); + if (status) + return mgmt_cmd_status(sk, hdev->id, op, status); + + return mgmt_cmd_complete(sk, hdev->id, op, MGMT_STATUS_SUCCESS, &rp, + sizeof(rp)); } static void parse_adv_monitor_rssi(struct adv_monitor *m, @@ -4817,7 +4762,7 @@ static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev, { struct mgmt_cp_add_adv_patterns_monitor *cp = data; struct adv_monitor *m = NULL; - u8 status = MGMT_STATUS_SUCCESS; + int status = MGMT_STATUS_SUCCESS; size_t expected_size = sizeof(*cp); BT_DBG("request for %s", hdev->name); @@ -4843,10 +4788,14 @@ static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev, parse_adv_monitor_rssi(m, NULL); status = parse_adv_monitor_pattern(m, cp->pattern_count, cp->patterns); + if (status) + goto done; + + status = __add_adv_patterns_monitor(sk, hdev, m, data, len, + MGMT_OP_ADD_ADV_PATTERNS_MONITOR); done: - return __add_adv_patterns_monitor(sk, hdev, m, status, data, len, - MGMT_OP_ADD_ADV_PATTERNS_MONITOR); + return status; } static int add_adv_patterns_monitor_rssi(struct sock *sk, struct hci_dev *hdev, @@ -4854,7 +4803,7 @@ static int add_adv_patterns_monitor_rssi(struct sock *sk, struct hci_dev *hdev, { struct mgmt_cp_add_adv_patterns_monitor_rssi *cp = data; struct adv_monitor *m = NULL; - u8 status = MGMT_STATUS_SUCCESS; + int status = MGMT_STATUS_SUCCESS; size_t expected_size = sizeof(*cp); BT_DBG("request for %s", hdev->name); @@ -4880,10 +4829,14 @@ static int add_adv_patterns_monitor_rssi(struct sock *sk, struct hci_dev *hdev, parse_adv_monitor_rssi(m, &cp->rssi); status = parse_adv_monitor_pattern(m, cp->pattern_count, cp->patterns); + if (status) + goto done; -done: - return __add_adv_patterns_monitor(sk, hdev, m, status, data, len, + status = __add_adv_patterns_monitor(sk, hdev, m, data, len, MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI); + +done: + return status; } int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status) @@ -4933,9 +4886,7 @@ static int remove_adv_monitor(struct sock *sk, struct hci_dev *hdev, hci_dev_lock(hdev); if (pending_find(MGMT_OP_SET_LE, hdev) || - pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev) || - pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR, hdev) || - pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI, hdev)) { + pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev)) { status = MGMT_STATUS_BUSY; goto unlock; } diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c index f43994523b1f..9abea16c4305 100644 --- a/net/bluetooth/msft.c +++ b/net/bluetooth/msft.c @@ -106,8 +106,6 @@ struct msft_data { __u8 filter_enabled; }; -static int __msft_add_monitor_pattern(struct hci_dev *hdev, - struct adv_monitor *monitor); static int __msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, u16 handle); @@ -164,34 +162,6 @@ static bool read_supported_features(struct hci_dev *hdev, return false; } -static void reregister_monitor(struct hci_dev *hdev, int handle) -{ - struct adv_monitor *monitor; - struct msft_data *msft = hdev->msft_data; - int err; - - while (1) { - monitor = idr_get_next(&hdev->adv_monitors_idr, &handle); - if (!monitor) { - /* All monitors have been resumed */ - msft->resuming = false; - hci_update_passive_scan(hdev); - return; - } - - msft->pending_add_handle = (u16)handle; - err = __msft_add_monitor_pattern(hdev, monitor); - - /* If success, we return and wait for monitor added callback */ - if (!err) - return; - - /* Otherwise remove the monitor and keep registering */ - hci_free_adv_monitor(hdev, monitor); - handle++; - } -} - /* is_mgmt = true matches the handle exposed to userspace via mgmt. * is_mgmt = false matches the handle used by the msft controller. * This function requires the caller holds hdev->lock @@ -243,14 +213,14 @@ static int msft_monitor_device_del(struct hci_dev *hdev, __u16 mgmt_handle, return count; } -static void msft_le_monitor_advertisement_cb(struct hci_dev *hdev, - u8 status, u16 opcode, - struct sk_buff *skb) +static int msft_le_monitor_advertisement_cb(struct hci_dev *hdev, u16 opcode, + struct sk_buff *skb) { struct msft_rp_le_monitor_advertisement *rp; struct adv_monitor *monitor; struct msft_monitor_advertisement_handle_data *handle_data; struct msft_data *msft = hdev->msft_data; + int status = 0; hci_dev_lock(hdev); @@ -262,15 +232,16 @@ static void msft_le_monitor_advertisement_cb(struct hci_dev *hdev, goto unlock; } - if (status) - goto unlock; - rp = (struct msft_rp_le_monitor_advertisement *)skb->data; if (skb->len < sizeof(*rp)) { status = HCI_ERROR_UNSPECIFIED; goto unlock; } + status = rp->status; + if (status) + goto unlock; + handle_data = kmalloc(sizeof(*handle_data), GFP_KERNEL); if (!handle_data) { status = HCI_ERROR_UNSPECIFIED; @@ -290,8 +261,7 @@ static void msft_le_monitor_advertisement_cb(struct hci_dev *hdev, hci_dev_unlock(hdev); - if (!msft->resuming) - hci_add_adv_patterns_monitor_complete(hdev, status); + return status; } static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev, @@ -463,7 +433,7 @@ static int msft_add_monitor_sync(struct hci_dev *hdev, ptrdiff_t offset = 0; u8 pattern_count = 0; struct sk_buff *skb; - u8 status; + struct msft_data *msft = hdev->msft_data; if (!msft_monitor_pattern_valid(monitor)) return -EINVAL; @@ -505,20 +475,40 @@ static int msft_add_monitor_sync(struct hci_dev *hdev, if (IS_ERR(skb)) return PTR_ERR(skb); - status = skb->data[0]; - skb_pull(skb, 1); + msft->pending_add_handle = monitor->handle; - msft_le_monitor_advertisement_cb(hdev, status, hdev->msft_opcode, skb); + return msft_le_monitor_advertisement_cb(hdev, hdev->msft_opcode, skb); +} - return status; +static void reregister_monitor(struct hci_dev *hdev) +{ + struct adv_monitor *monitor; + struct msft_data *msft = hdev->msft_data; + int handle = 0; + + if (!msft) + return; + + msft->resuming = true; + + while (1) { + monitor = idr_get_next(&hdev->adv_monitors_idr, &handle); + if (!monitor) + break; + + msft_add_monitor_sync(hdev, monitor); + + handle++; + } + + /* All monitors have been reregistered */ + msft->resuming = false; } /* This function requires the caller holds hci_req_sync_lock */ int msft_resume_sync(struct hci_dev *hdev) { struct msft_data *msft = hdev->msft_data; - struct adv_monitor *monitor; - int handle = 0; if (!msft || !msft_monitor_supported(hdev)) return 0; @@ -533,24 +523,12 @@ int msft_resume_sync(struct hci_dev *hdev) hci_dev_unlock(hdev); - msft->resuming = true; - - while (1) { - monitor = idr_get_next(&hdev->adv_monitors_idr, &handle); - if (!monitor) - break; - - msft_add_monitor_sync(hdev, monitor); - - handle++; - } - - /* All monitors have been resumed */ - msft->resuming = false; + reregister_monitor(hdev); return 0; } +/* This function requires the caller holds hci_req_sync_lock */ void msft_do_open(struct hci_dev *hdev) { struct msft_data *msft = hdev->msft_data; @@ -583,7 +561,7 @@ void msft_do_open(struct hci_dev *hdev) /* Monitors get removed on power off, so we need to explicitly * tell the controller to re-monitor. */ - reregister_monitor(hdev, 0); + reregister_monitor(hdev); } } @@ -829,66 +807,7 @@ static void msft_le_set_advertisement_filter_enable_cb(struct hci_dev *hdev, hci_dev_unlock(hdev); } -/* This function requires the caller holds hdev->lock */ -static int __msft_add_monitor_pattern(struct hci_dev *hdev, - struct adv_monitor *monitor) -{ - struct msft_cp_le_monitor_advertisement *cp; - struct msft_le_monitor_advertisement_pattern_data *pattern_data; - struct msft_le_monitor_advertisement_pattern *pattern; - struct adv_pattern *entry; - struct hci_request req; - struct msft_data *msft = hdev->msft_data; - size_t total_size = sizeof(*cp) + sizeof(*pattern_data); - ptrdiff_t offset = 0; - u8 pattern_count = 0; - int err = 0; - - if (!msft_monitor_pattern_valid(monitor)) - return -EINVAL; - - list_for_each_entry(entry, &monitor->patterns, list) { - pattern_count++; - total_size += sizeof(*pattern) + entry->length; - } - - cp = kmalloc(total_size, GFP_KERNEL); - if (!cp) - return -ENOMEM; - - cp->sub_opcode = MSFT_OP_LE_MONITOR_ADVERTISEMENT; - cp->rssi_high = monitor->rssi.high_threshold; - cp->rssi_low = monitor->rssi.low_threshold; - cp->rssi_low_interval = (u8)monitor->rssi.low_threshold_timeout; - cp->rssi_sampling_period = monitor->rssi.sampling_period; - - cp->cond_type = MSFT_MONITOR_ADVERTISEMENT_TYPE_PATTERN; - - pattern_data = (void *)cp->data; - pattern_data->count = pattern_count; - - list_for_each_entry(entry, &monitor->patterns, list) { - pattern = (void *)(pattern_data->data + offset); - /* the length also includes data_type and offset */ - pattern->length = entry->length + 2; - pattern->data_type = entry->ad_type; - pattern->start_byte = entry->offset; - memcpy(pattern->pattern, entry->value, entry->length); - offset += sizeof(*pattern) + entry->length; - } - - hci_req_init(&req, hdev); - hci_req_add(&req, hdev->msft_opcode, total_size, cp); - err = hci_req_run_skb(&req, msft_le_monitor_advertisement_cb); - kfree(cp); - - if (!err) - msft->pending_add_handle = monitor->handle; - - return err; -} - -/* This function requires the caller holds hdev->lock */ +/* This function requires the caller holds hci_req_sync_lock */ int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor) { struct msft_data *msft = hdev->msft_data; @@ -899,7 +818,7 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor) if (msft->resuming || msft->suspending) return -EBUSY; - return __msft_add_monitor_pattern(hdev, monitor); + return msft_add_monitor_sync(hdev, monitor); } /* This function requires the caller holds hdev->lock */ From patchwork Tue May 24 20:14:25 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Manish Mandlik X-Patchwork-Id: 577017 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 27772C433F5 for ; Tue, 24 May 2022 20:14:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238285AbiEXUOj (ORCPT ); Tue, 24 May 2022 16:14:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47596 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241445AbiEXUOc (ORCPT ); Tue, 24 May 2022 16:14:32 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B5E6A82165 for ; Tue, 24 May 2022 13:14:30 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id l72-20020a25254b000000b00651f60988dfso2998372ybl.11 for ; Tue, 24 May 2022 13:14:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=21cS6HrQlto5xKVBaRyielngkbD0PbPrl0cGSh2e7BE=; b=VVa8TWwbnWP3kVBlGBWhzeLb1P+zhQIoAx3I1uI5oV5drao02Vtq2WN65Hq9Trsnfv FDqkv0aNyxOqGee0MwC1kQ5KPR1qhJtQYMTLVIaZRjBCEvSg3pwDnEl4j7j9l5c4uo1E iNWRCuihAWFvP/vZZ5M4AGcTanLYNduxuIaLINLlN/795cHxqpQrm8Po6C063XYtgWtZ u+f3QtWVXCEC5gsZQ9eXiot5jt8A8z2q3oQiHZZ8vViZ61iqXUTXNe6fhvXu7HUxeLcz BRRZE+PUJM9ZwqDajBr5wUP/G6+haBU0pylaBBzo9PLnPQ111KHKetiKBRM0dWJ1vNEf FUYQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=21cS6HrQlto5xKVBaRyielngkbD0PbPrl0cGSh2e7BE=; b=5zMmEqbuJakWNTz6kV5I7PFD2E2XiCWe7GN68Ggk8m0X3NdaQrGsCD/gjC/J4Xfyhi bZCvY6vJzXUBNOZn7QfAxYcm6TNTVBe86d/kPsyoGpdKV2+q8M7+Q1XCJIQJio8fTJA2 BnRlGxeSODsR9ll5mUb4tykM8KmxDR0areOMVWTdkp9XwC9YC/LugsFbNi1K4QeDFM0Q O3+BXbY+CFfNYMUJMmz4AVxXZRm6tRmPWxn8IlSK1CORoFtOwPAfrEiOK+XjkYjbHIpW 8MUaFCNhd9TUnaR6ebZ1yExl2sLPXe6LZaOcOClEylwiYlPET6mTxKYvwz4L4DGGH8DO xyxA== X-Gm-Message-State: AOAM532UOarqwF43T4lviXNgD5G4JNDgVleITns6jwqJAl0QXK3+Vm3L 7Lu/sv5ybvpt7bbwF4mdx3RmfTIhBpkBSQ== X-Google-Smtp-Source: ABdhPJz8WHwRNrKCBAec4wIS6xJuhRvYSntcxbf8ukckt6s2KLWTDq+JRpmXuPe8tDBm55b9pipVIHust5tv6Q== X-Received: from mmandlik.mtv.corp.google.com ([2620:15c:202:201:867d:d253:fd17:d811]) (user=mmandlik job=sendgmr) by 2002:a5b:302:0:b0:64b:a20a:fcd9 with SMTP id j2-20020a5b0302000000b0064ba20afcd9mr27418093ybp.492.1653423269940; Tue, 24 May 2022 13:14:29 -0700 (PDT) Date: Tue, 24 May 2022 13:14:25 -0700 In-Reply-To: <20220524131406.1.If745ed1d05d98c002fc84ba60cef99eb786b7caa@changeid> Message-Id: <20220524131407.2.Id703da51f33c425056a1148b91468dd6b06429b3@changeid> Mime-Version: 1.0 References: <20220524131406.1.If745ed1d05d98c002fc84ba60cef99eb786b7caa@changeid> X-Mailer: git-send-email 2.36.1.124.g0e6072fb45-goog Subject: [PATCH 2/2] Bluetooth: hci_sync: Refactor remove Adv Monitor From: Manish Mandlik To: marcel@holtmann.org, luiz.dentz@gmail.com Cc: chromeos-bluetooth-upstreaming@chromium.org, linux-bluetooth@vger.kernel.org, Manish Mandlik , Miao-chen Chou , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Johan Hedberg , Paolo Abeni , linux-kernel@vger.kernel.org, netdev@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Make use of hci_cmd_sync_queue for removing an advertisement monitor. Signed-off-by: Manish Mandlik Reviewed-by: Miao-chen Chou --- include/net/bluetooth/hci_core.h | 6 +-- net/bluetooth/hci_core.c | 87 ++++++++++---------------------- net/bluetooth/mgmt.c | 67 ++++++------------------ net/bluetooth/msft.c | 87 +++++++------------------------- net/bluetooth/msft.h | 6 +-- 5 files changed, 63 insertions(+), 190 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 59953a7a6328..7a1e48d794ea 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1410,10 +1410,9 @@ bool hci_adv_instance_is_scannable(struct hci_dev *hdev, u8 instance); void hci_adv_monitors_clear(struct hci_dev *hdev); void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor); -int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status); int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor); -bool hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle, int *err); -bool hci_remove_all_adv_monitor(struct hci_dev *hdev, int *err); +int hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle); +int hci_remove_all_adv_monitor(struct hci_dev *hdev); bool hci_is_adv_monitoring(struct hci_dev *hdev); int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev); @@ -1873,7 +1872,6 @@ void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev, u8 instance); void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle); int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip); -int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status); void mgmt_adv_monitor_device_lost(struct hci_dev *hdev, u16 handle, bdaddr_t *bdaddr, u8 addr_type); diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index bbbbe3203130..c233844a3fc4 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -1873,11 +1873,6 @@ void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor) kfree(monitor); } -int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status) -{ - return mgmt_remove_adv_monitor_complete(hdev, status); -} - /* Assigns handle to a monitor, and if offloading is supported and power is on, * also attempts to forward the request to the controller. */ @@ -1927,92 +1922,64 @@ int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor) /* Attempts to tell the controller and free the monitor. If somehow the * controller doesn't have a corresponding handle, remove anyway. - * Returns true if request is forwarded (result is pending), false otherwise. - * This function requires the caller holds hdev->lock. */ -static bool hci_remove_adv_monitor(struct hci_dev *hdev, - struct adv_monitor *monitor, - u16 handle, int *err) +static int hci_remove_adv_monitor(struct hci_dev *hdev, + struct adv_monitor *monitor) { - *err = 0; + int status = 0; switch (hci_get_adv_monitor_offload_ext(hdev)) { case HCI_ADV_MONITOR_EXT_NONE: /* also goes here when powered off */ - goto free_monitor; + hci_free_adv_monitor(hdev, monitor); + bt_dev_dbg(hdev, "%s remove monitor %d status %d", hdev->name, + monitor->handle, status); + break; + case HCI_ADV_MONITOR_EXT_MSFT: - *err = msft_remove_monitor(hdev, monitor, handle); + hci_req_sync_lock(hdev); + status = msft_remove_monitor(hdev, monitor); + hci_req_sync_unlock(hdev); + bt_dev_dbg(hdev, "%s remove monitor %d msft status %d", + hdev->name, monitor->handle, status); break; } - /* In case no matching handle registered, just free the monitor */ - if (*err == -ENOENT) - goto free_monitor; - - return (*err == 0); - -free_monitor: - if (*err == -ENOENT) + if (status == -ENOENT) bt_dev_warn(hdev, "Removing monitor with no matching handle %d", monitor->handle); - hci_free_adv_monitor(hdev, monitor); - *err = 0; - return false; + return status; } -/* Returns true if request is forwarded (result is pending), false otherwise. - * This function requires the caller holds hdev->lock. - */ -bool hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle, int *err) +int hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle) { struct adv_monitor *monitor = idr_find(&hdev->adv_monitors_idr, handle); - bool pending; - - if (!monitor) { - *err = -EINVAL; - return false; - } - pending = hci_remove_adv_monitor(hdev, monitor, handle, err); - if (!*err && !pending) - hci_update_passive_scan(hdev); - - bt_dev_dbg(hdev, "%s remove monitor handle %d, status %d, %spending", - hdev->name, handle, *err, pending ? "" : "not "); + if (!monitor) + return -EINVAL; - return pending; + return hci_remove_adv_monitor(hdev, monitor); } -/* Returns true if request is forwarded (result is pending), false otherwise. - * This function requires the caller holds hdev->lock. - */ -bool hci_remove_all_adv_monitor(struct hci_dev *hdev, int *err) +int hci_remove_all_adv_monitor(struct hci_dev *hdev) { struct adv_monitor *monitor; int idr_next_id = 0; - bool pending = false; - bool update = false; - - *err = 0; + int status = 0; - while (!*err && !pending) { + while (1) { monitor = idr_get_next(&hdev->adv_monitors_idr, &idr_next_id); if (!monitor) break; - pending = hci_remove_adv_monitor(hdev, monitor, 0, err); + status = hci_remove_adv_monitor(hdev, monitor); + if (status) + return status; - if (!*err && !pending) - update = true; + idr_next_id++; } - if (update) - hci_update_passive_scan(hdev); - - bt_dev_dbg(hdev, "%s remove all monitors status %d, %spending", - hdev->name, *err, pending ? "" : "not "); - - return pending; + return status; } /* This function requires the caller holds hdev->lock */ diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index d04f90698a87..12d91cd87ff0 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -4839,37 +4839,6 @@ static int add_adv_patterns_monitor_rssi(struct sock *sk, struct hci_dev *hdev, return status; } -int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status) -{ - struct mgmt_rp_remove_adv_monitor rp; - struct mgmt_cp_remove_adv_monitor *cp; - struct mgmt_pending_cmd *cmd; - int err = 0; - - hci_dev_lock(hdev); - - cmd = pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev); - if (!cmd) - goto done; - - cp = cmd->param; - rp.monitor_handle = cp->monitor_handle; - - if (!status) - hci_update_passive_scan(hdev); - - err = mgmt_cmd_complete(cmd->sk, cmd->index, cmd->opcode, - mgmt_status(status), &rp, sizeof(rp)); - mgmt_pending_remove(cmd); - -done: - hci_dev_unlock(hdev); - bt_dev_dbg(hdev, "remove monitor %d complete, status %u", - rp.monitor_handle, status); - - return err; -} - static int remove_adv_monitor(struct sock *sk, struct hci_dev *hdev, void *data, u16 len) { @@ -4877,11 +4846,7 @@ static int remove_adv_monitor(struct sock *sk, struct hci_dev *hdev, struct mgmt_rp_remove_adv_monitor rp; struct mgmt_pending_cmd *cmd; u16 handle = __le16_to_cpu(cp->monitor_handle); - int err, status; - bool pending; - - BT_DBG("request for %s", hdev->name); - rp.monitor_handle = cp->monitor_handle; + int err, status = MGMT_STATUS_SUCCESS; hci_dev_lock(hdev); @@ -4897,15 +4862,19 @@ static int remove_adv_monitor(struct sock *sk, struct hci_dev *hdev, goto unlock; } + hci_dev_unlock(hdev); + if (handle) - pending = hci_remove_single_adv_monitor(hdev, handle, &err); + err = hci_remove_single_adv_monitor(hdev, handle); else - pending = hci_remove_all_adv_monitor(hdev, &err); + err = hci_remove_all_adv_monitor(hdev); - if (err) { - mgmt_pending_remove(cmd); + hci_dev_lock(hdev); + + mgmt_pending_remove(cmd); - if (err == -ENOENT) + if (err) { + if (err == -ENOENT || err == -EINVAL) status = MGMT_STATUS_INVALID_INDEX; else status = MGMT_STATUS_FAILED; @@ -4913,19 +4882,13 @@ static int remove_adv_monitor(struct sock *sk, struct hci_dev *hdev, goto unlock; } - /* monitor can be removed without forwarding request to controller */ - if (!pending) { - mgmt_pending_remove(cmd); - hci_dev_unlock(hdev); - - return mgmt_cmd_complete(sk, hdev->id, - MGMT_OP_REMOVE_ADV_MONITOR, - MGMT_STATUS_SUCCESS, - &rp, sizeof(rp)); - } + rp.monitor_handle = cp->monitor_handle; + hci_update_passive_scan(hdev); hci_dev_unlock(hdev); - return 0; + + return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_REMOVE_ADV_MONITOR, + MGMT_STATUS_SUCCESS, &rp, sizeof(rp)); unlock: hci_dev_unlock(hdev); diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c index 9abea16c4305..0d3378e707db 100644 --- a/net/bluetooth/msft.c +++ b/net/bluetooth/msft.c @@ -106,9 +106,6 @@ struct msft_data { __u8 filter_enabled; }; -static int __msft_remove_monitor(struct hci_dev *hdev, - struct adv_monitor *monitor, u16 handle); - bool msft_monitor_supported(struct hci_dev *hdev) { return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR); @@ -264,20 +261,16 @@ static int msft_le_monitor_advertisement_cb(struct hci_dev *hdev, u16 opcode, return status; } -static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev, - u8 status, u16 opcode, - struct sk_buff *skb) +static int msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev, + u16 opcode, + struct sk_buff *skb) { struct msft_cp_le_cancel_monitor_advertisement *cp; struct msft_rp_le_cancel_monitor_advertisement *rp; struct adv_monitor *monitor; struct msft_monitor_advertisement_handle_data *handle_data; struct msft_data *msft = hdev->msft_data; - int err; - bool pending; - - if (status) - goto done; + int status = 0; rp = (struct msft_rp_le_cancel_monitor_advertisement *)skb->data; if (skb->len < sizeof(*rp)) { @@ -285,6 +278,10 @@ static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev, goto done; } + status = rp->status; + if (status) + goto done; + hci_dev_lock(hdev); cp = hci_sent_cmd_data(hdev, hdev->msft_opcode); @@ -312,26 +309,10 @@ static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev, kfree(handle_data); } - /* If remove all monitors is required, we need to continue the process - * here because the earlier it was paused when waiting for the - * response from controller. - */ - if (msft->pending_remove_handle == 0) { - pending = hci_remove_all_adv_monitor(hdev, &err); - if (pending) { - hci_dev_unlock(hdev); - return; - } - - if (err) - status = HCI_ERROR_UNSPECIFIED; - } - hci_dev_unlock(hdev); done: - if (!msft->suspending) - hci_remove_adv_monitor_complete(hdev, status); + return status; } static int msft_remove_monitor_sync(struct hci_dev *hdev, @@ -340,13 +321,14 @@ static int msft_remove_monitor_sync(struct hci_dev *hdev, struct msft_cp_le_cancel_monitor_advertisement cp; struct msft_monitor_advertisement_handle_data *handle_data; struct sk_buff *skb; - u8 status; handle_data = msft_find_handle_data(hdev, monitor->handle, true); /* If no matched handle, just remove without telling controller */ - if (!handle_data) + if (!handle_data) { + hci_free_adv_monitor(hdev, monitor); return -ENOENT; + } cp.sub_opcode = MSFT_OP_LE_CANCEL_MONITOR_ADVERTISEMENT; cp.handle = handle_data->msft_handle; @@ -356,13 +338,8 @@ static int msft_remove_monitor_sync(struct hci_dev *hdev, if (IS_ERR(skb)) return PTR_ERR(skb); - status = skb->data[0]; - skb_pull(skb, 1); - - msft_le_cancel_monitor_advertisement_cb(hdev, status, hdev->msft_opcode, - skb); - - return status; + return msft_le_cancel_monitor_advertisement_cb(hdev, hdev->msft_opcode, + skb); } /* This function requires the caller holds hci_req_sync_lock */ @@ -821,38 +798,8 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor) return msft_add_monitor_sync(hdev, monitor); } -/* This function requires the caller holds hdev->lock */ -static int __msft_remove_monitor(struct hci_dev *hdev, - struct adv_monitor *monitor, u16 handle) -{ - struct msft_cp_le_cancel_monitor_advertisement cp; - struct msft_monitor_advertisement_handle_data *handle_data; - struct hci_request req; - struct msft_data *msft = hdev->msft_data; - int err = 0; - - handle_data = msft_find_handle_data(hdev, monitor->handle, true); - - /* If no matched handle, just remove without telling controller */ - if (!handle_data) - return -ENOENT; - - cp.sub_opcode = MSFT_OP_LE_CANCEL_MONITOR_ADVERTISEMENT; - cp.handle = handle_data->msft_handle; - - hci_req_init(&req, hdev); - hci_req_add(&req, hdev->msft_opcode, sizeof(cp), &cp); - err = hci_req_run_skb(&req, msft_le_cancel_monitor_advertisement_cb); - - if (!err) - msft->pending_remove_handle = handle; - - return err; -} - -/* This function requires the caller holds hdev->lock */ -int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, - u16 handle) +/* This function requires the caller holds hci_req_sync_lock */ +int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor) { struct msft_data *msft = hdev->msft_data; @@ -862,7 +809,7 @@ int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, if (msft->resuming || msft->suspending) return -EBUSY; - return __msft_remove_monitor(hdev, monitor, handle); + return msft_remove_monitor_sync(hdev, monitor); } void msft_req_add_set_filter_enable(struct hci_request *req, bool enable) diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h index afcaf7d3b1cb..2a63205b377b 100644 --- a/net/bluetooth/msft.h +++ b/net/bluetooth/msft.h @@ -20,8 +20,7 @@ void msft_do_close(struct hci_dev *hdev); void msft_vendor_evt(struct hci_dev *hdev, void *data, struct sk_buff *skb); __u64 msft_get_features(struct hci_dev *hdev); int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor); -int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, - u16 handle); +int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor); void msft_req_add_set_filter_enable(struct hci_request *req, bool enable); int msft_set_filter_enable(struct hci_dev *hdev, bool enable); int msft_suspend_sync(struct hci_dev *hdev); @@ -49,8 +48,7 @@ static inline int msft_add_monitor_pattern(struct hci_dev *hdev, } static inline int msft_remove_monitor(struct hci_dev *hdev, - struct adv_monitor *monitor, - u16 handle) + struct adv_monitor *monitor) { return -EOPNOTSUPP; }