Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210303192012.1695032-1-danielwinkler@google.com> <20210303111817.Bluez.2.I45b896f4512038309cbeab7a01f51e503141edab@changeid>
In-Reply-To: <20210303111817.Bluez.2.I45b896f4512038309cbeab7a01f51e503141edab@changeid>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Thu, 4 Mar 2021 10:59:31 -0800
Message-ID: <CABBYNZ+NtxCwE5XWwKEGOXtyvgb4GUm0A4Pstwj2sOd9fVx8Gw@mail.gmail.com>
Subject: Re: [Bluez PATCH 2/3] advertising: Create and use scannable adv param flag
To:     Daniel Winkler <danielwinkler@google.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        ChromeOS Bluetooth Upstreaming 
        <chromeos-bluetooth-upstreaming@chromium.org>,
        Alain Michaud <alainm@chromium.org>,
        Sonny Sasaka <sonnysasaka@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Daniel,

On Wed, Mar 3, 2021 at 11:20 AM Daniel Winkler <danielwinkler@google.com> wrote:
>
> In order for the advertising parameters hci request to indicate that an
> advertising set uses a scannable PDU, we pass a scannable flag along
> with the initial parameters MGMT request.
>
> Without this patch, a broadcast advertisement with a scan response will
> either be rejected by the controller, or will ignore the requested scan
> response. The patch is tested by performing the above and confirming
> that the scan response is retrievable from a peer as expected.
>
> Reviewed-by: Alain Michaud <alainm@chromium.org>
> Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
>
> ---
>
>  lib/mgmt.h        | 1 +
>  src/advertising.c | 4 ++++
>  2 files changed, 5 insertions(+)
>
> diff --git a/lib/mgmt.h b/lib/mgmt.h
> index 76a03c9c2..7b1b9ab54 100644
> --- a/lib/mgmt.h
> +++ b/lib/mgmt.h
> @@ -507,6 +507,7 @@ struct mgmt_rp_add_advertising {
>  #define MGMT_ADV_PARAM_TIMEOUT         (1 << 13)
>  #define MGMT_ADV_PARAM_INTERVALS       (1 << 14)
>  #define MGMT_ADV_PARAM_TX_POWER                (1 << 15)
> +#define MGMT_ADV_PARAM_SCAN_RSP                (1 << 16)
>
>  #define MGMT_OP_REMOVE_ADVERTISING     0x003F
>  struct mgmt_cp_remove_advertising {
> diff --git a/src/advertising.c b/src/advertising.c
> index f3dc357a1..38cef565f 100644
> --- a/src/advertising.c
> +++ b/src/advertising.c
> @@ -945,6 +945,10 @@ static int refresh_extended_adv(struct btd_adv_client *client,
>                 return -EINVAL;
>         }
>
> +       /* Indicate that this instance will be configured as scannable */
> +       if (client->scan_rsp_len)
> +               flags |= MGMT_ADV_PARAM_SCAN_RSP;
> +

Don't we need to check if the flag is actually supported by the kernel?

>         cp.flags = htobl(flags);

For new code it is prefered to use the function from src/shared/util.h
(cpu_to_*).

>         mgmt_ret = mgmt_send(client->manager->mgmt, MGMT_OP_ADD_EXT_ADV_PARAMS,
> --
> 2.30.1.766.gb4fecdf3b7-goog
>


-- 
Luiz Augusto von Dentz