Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200407205217.1002534-1-marcel@holtmann.org>
In-Reply-To: <20200407205217.1002534-1-marcel@holtmann.org>
From:   Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Date:   Tue, 7 Apr 2020 16:37:28 -0700
Message-ID: <CANFp7mVSQEOyT7GrOa4NhAGozt5SMx3yU_ma6tav5bJ8GXJn8w@mail.gmail.com>
Subject: Re: [PATCH v3] Bluetooth: Configure controller address resolution if available
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     Bluez mailing list <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Marcel,

This looks good to me.

Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>


On Tue, Apr 7, 2020 at 1:52 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> When the LL Privacy support is available, then as part of enabling or
> disabling passive background scanning, it is required to set up the
> controller based address resolution as well.
>
> Since only passive background scanning is utilizing the whitelist, the
> address resolution is now bound to the whitelist and passive background
> scanning. All other resolution can be easily done by the host stack.
>
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  include/net/bluetooth/hci.h      |  1 +
>  include/net/bluetooth/hci_core.h |  4 ++++
>  net/bluetooth/hci_request.c      | 26 +++++++++++++++++++++++---
>  3 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 5ef4547760db..58360538d42b 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -459,6 +459,7 @@ enum {
>  #define HCI_LE_SLAVE_FEATURES          0x08
>  #define HCI_LE_PING                    0x10
>  #define HCI_LE_DATA_LEN_EXT            0x20
> +#define HCI_LE_LL_PRIVACY              0x40
>  #define HCI_LE_EXT_SCAN_POLICY         0x80
>  #define HCI_LE_PHY_2M                  0x01
>  #define HCI_LE_PHY_CODED               0x08
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 2f3275f1d1c4..663ffde9bd1d 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -645,6 +645,7 @@ extern struct mutex hci_cb_list_lock;
>         do {                                                    \
>                 hci_dev_clear_flag(hdev, HCI_LE_SCAN);          \
>                 hci_dev_clear_flag(hdev, HCI_LE_ADV);           \
> +               hci_dev_clear_flag(hdev, HCI_LL_RPA_RESOLUTION);\
>                 hci_dev_clear_flag(hdev, HCI_PERIODIC_INQ);     \
>         } while (0)
>
> @@ -1277,6 +1278,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>  #define scan_coded(dev) (((dev)->le_tx_def_phys & HCI_LE_SET_PHY_CODED) || \
>                          ((dev)->le_rx_def_phys & HCI_LE_SET_PHY_CODED))
>
> +/* Use LL Privacy based address resolution if supported */
> +#define use_ll_privacy(dev) ((dev)->le_features[0] & HCI_LE_LL_PRIVACY)
> +
>  /* Use ext scanning if set ext scan param and ext scan enable is supported */
>  #define use_ext_scan(dev) (((dev)->commands[37] & 0x20) && \
>                            ((dev)->commands[37] & 0x40))
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index 9ea40106ef17..efec2a0bb824 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -676,6 +676,12 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
>                 cp.enable = LE_SCAN_DISABLE;
>                 hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
>         }
> +
> +       if (use_ll_privacy(hdev) &&
> +           hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION)) {
> +               __u8 enable = 0x00;
> +               hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE, 1, &enable);
> +       }
>  }
>
>  static void del_from_white_list(struct hci_request *req, bdaddr_t *bdaddr,
> @@ -808,10 +814,16 @@ static bool scan_use_rpa(struct hci_dev *hdev)
>  }
>
>  static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
> -                              u16 window, u8 own_addr_type, u8 filter_policy)
> +                              u16 window, u8 own_addr_type, u8 filter_policy,
> +                              bool addr_resolv)
>  {
>         struct hci_dev *hdev = req->hdev;
>
> +       if (use_ll_privacy(hdev) && addr_resolv) {
> +               __u8 enable = 0x01;
> +               hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE, 1, &enable);
> +       }
> +
>         /* Use ext scanning if set ext scan param and ext scan enable is
>          * supported
>          */
> @@ -885,12 +897,18 @@ static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
>         }
>  }
>
> +/* Ensure to call hci_req_add_le_scan_disable() first to disable the
> + * controller based address resolution to be able to reconfigure
> + * resolving list.
> + */
>  void hci_req_add_le_passive_scan(struct hci_request *req)
>  {
>         struct hci_dev *hdev = req->hdev;
>         u8 own_addr_type;
>         u8 filter_policy;
>         u8 window, interval;
> +       /* Background scanning should run with address resolution */
> +       bool addr_resolv = true;
>
>         if (hdev->scanning_paused) {
>                 bt_dev_dbg(hdev, "Scanning is paused for suspend");
> @@ -936,7 +954,7 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
>
>         bt_dev_dbg(hdev, "LE passive scan with whitelist = %d", filter_policy);
>         hci_req_start_scan(req, LE_SCAN_PASSIVE, interval, window,
> -                          own_addr_type, filter_policy);
> +                          own_addr_type, filter_policy, addr_resolv);
>  }
>
>  static u8 get_adv_instance_scan_rsp_len(struct hci_dev *hdev, u8 instance)
> @@ -2725,6 +2743,8 @@ static int active_scan(struct hci_request *req, unsigned long opt)
>         u8 own_addr_type;
>         /* White list is not used for discovery */
>         u8 filter_policy = 0x00;
> +       /* Discovery doesn't require controller address resolution */
> +       bool addr_resolv = false;
>         int err;
>
>         BT_DBG("%s", hdev->name);
> @@ -2746,7 +2766,7 @@ static int active_scan(struct hci_request *req, unsigned long opt)
>                 own_addr_type = ADDR_LE_DEV_PUBLIC;
>
>         hci_req_start_scan(req, LE_SCAN_ACTIVE, interval, DISCOV_LE_SCAN_WIN,
> -                          own_addr_type, filter_policy);
> +                          own_addr_type, filter_policy, addr_resolv);
>         return 0;
>  }
>
> --
> 2.25.2
>