All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: Daniel Winkler <danielwinkler@google.com>
Cc: Luiz Augusto Von Dentz <luiz.von.dentz@intel.com>,
	"linux-bluetooth@vger.kernel.org"
	<linux-bluetooth@vger.kernel.org>,
	ChromeOS Bluetooth Upstreaming 
	<chromeos-bluetooth-upstreaming@chromium.org>,
	Sonny Sasaka <sonnysasaka@chromium.org>,
	Alain Michaud <alainm@chromium.org>
Subject: Re: [Bluez PATCH v4 07/10] advertising: Query LE TX range at manager initialization
Date: Tue, 27 Oct 2020 15:08:11 -0700	[thread overview]
Message-ID: <CABBYNZK3SQeWp3OHSgapqL+HEZqcFuPFbwMFCiU9m-vAKd-1gQ@mail.gmail.com> (raw)
In-Reply-To: <20201001155738.Bluez.v4.7.I187f71748b9bd93f6bf97ec4a195216109c3ea06@changeid>

Hi Daniel,

On Thu, Oct 1, 2020 at 4:05 PM Daniel Winkler <danielwinkler@google.com> wrote:
>
> This patch calls the new MGMT command to get controller capabilities,
> and parses the min and max LE tx power range when the manager is
> initialized. This will be used to populate a client-facing dbus entry so
> that the client will know the advertising capabilities of the controller
> before registering an advertisement.
>
> This patch is tested by manually verifying the data is parsed correctly
> from the MGMT response.
>
> Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
> Reviewed-by: Alain Michaud <alainm@chromium.org>
> ---
>
> Changes in v4:
> - Move tx power range into single capability field
>
> Changes in v3: None
> Changes in v2: None
>
>  lib/mgmt.h        | 14 ++++++++----
>  src/advertising.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
>  tools/btmgmt.c    | 12 +++++------
>  3 files changed, 70 insertions(+), 10 deletions(-)
>
> diff --git a/lib/mgmt.h b/lib/mgmt.h
> index 7ab4fb797..f37f7e654 100644
> --- a/lib/mgmt.h
> +++ b/lib/mgmt.h
> @@ -599,10 +599,16 @@ struct mgmt_cp_set_blocked_keys {
>         struct mgmt_blocked_key_info keys[0];
>  } __packed;
>
> -#define MGMT_OP_READ_SECURITY_INFO     0x0048
> -struct mgmt_rp_read_security_info {
> -       uint16_t sec_len;
> -       uint8_t  sec[0];
> +#define MGMT_CAP_SEC_FLAGS             0x01
> +#define MGMT_CAP_MAX_ENC_KEY_SIZE      0x02
> +#define MGMT_CAP_SMP_MAX_ENC_KEY_SIZE  0x03
> +#define MGMT_CAP_LE_TX_PWR             0x04
> +
> +#define MGMT_OP_READ_CONTROLLER_CAP    0x0048
> +#define MGMT_READ_CONTROLLER_CAP_SIZE  0
> +struct mgmt_rp_read_controller_cap {
> +       uint16_t cap_len;
> +       uint8_t cap[0];
>  } __packed;
>
>  #define MGMT_OP_READ_EXP_FEATURES_INFO 0x0049
> diff --git a/src/advertising.c b/src/advertising.c
> index 052964d42..41d0658c8 100644
> --- a/src/advertising.c
> +++ b/src/advertising.c
> @@ -49,6 +49,8 @@ struct btd_adv_manager {
>         uint32_t supported_flags;
>         unsigned int instance_bitmap;
>         bool extended_add_cmds;
> +       int8_t min_tx_power;
> +       int8_t max_tx_power;
>  };
>
>  #define AD_TYPE_BROADCAST 0
> @@ -1679,6 +1681,49 @@ static void read_adv_features_callback(uint8_t status, uint16_t length,
>                 remove_advertising(manager, 0);
>  }
>
> +static void read_controller_cap_complete(uint8_t status, uint16_t length,
> +                                       const void *param, void *user_data)
> +{
> +       struct btd_adv_manager *manager = user_data;
> +       const struct mgmt_rp_read_controller_cap *rp = param;
> +       const uint8_t *ptr = rp->cap;
> +       size_t offset = 0;
> +       uint8_t tag_len;
> +       uint8_t tag_type;
> +
> +       if (status || !param) {
> +               error("Failed to read advertising features: %s (0x%02x)",
> +                                               mgmt_errstr(status), status);
> +               return;
> +       }
> +
> +       if (sizeof(rp->cap_len) + rp->cap_len != length) {
> +               error("Controller capabilities malformed, size %lu != %u",
> +                               sizeof(rp->cap_len) + rp->cap_len, length);
> +               return;
> +       }
> +
> +       while (offset < rp->cap_len) {
> +               tag_len = ptr[offset++];
> +               tag_type = ptr[offset++];
> +
> +               switch (tag_type) {
> +               case MGMT_CAP_LE_TX_PWR:
> +                       if ((tag_len - sizeof(tag_type)) !=
> +                                       2*sizeof(manager->min_tx_power)) {
> +                               error("TX power had unexpected length %d",
> +                                       tag_len);
> +                               break;
> +                       }
> +                       memcpy(&manager->min_tx_power, &ptr[offset], tag_len);
> +                       memcpy(&manager->max_tx_power, &ptr[offset+1], tag_len);
> +               }
> +
> +               /* Step to the next entry */
> +               offset += (tag_len - sizeof(tag_type));
> +       }
> +}
> +
>  static struct btd_adv_manager *manager_create(struct btd_adapter *adapter,
>                                                 struct mgmt *mgmt)
>  {
> @@ -1700,6 +1745,8 @@ static struct btd_adv_manager *manager_create(struct btd_adapter *adapter,
>         manager->supported_flags = MGMT_ADV_FLAG_LOCAL_NAME;
>         manager->extended_add_cmds =
>                         btd_has_kernel_features(KERNEL_HAS_EXT_ADV_ADD_CMDS);
> +       manager->min_tx_power = ADV_TX_POWER_NO_PREFERENCE;
> +       manager->max_tx_power = ADV_TX_POWER_NO_PREFERENCE;
>
>         if (!g_dbus_register_interface(btd_get_dbus_connection(),
>                                         adapter_get_path(manager->adapter),
> @@ -1716,6 +1763,13 @@ static struct btd_adv_manager *manager_create(struct btd_adapter *adapter,
>                 goto fail;
>         }
>
> +       /* Query controller capabilities. This will be used to display valid
> +        * advertising tx power range to the client.
> +        */
> +       mgmt_send(manager->mgmt, MGMT_OP_READ_CONTROLLER_CAP,
> +               manager->mgmt_index, 0, NULL,
> +               read_controller_cap_complete, manager, NULL);

I'd make this conditional to experimental for now, also don't we need
to check the kernel has support for it?

>         return manager;
>
>  fail:
> diff --git a/tools/btmgmt.c b/tools/btmgmt.c
> index 48c9e5887..8b1cc4df5 100644
> --- a/tools/btmgmt.c
> +++ b/tools/btmgmt.c
> @@ -1531,7 +1531,7 @@ static void cmd_extinfo(int argc, char **argv)
>  static void sec_info_rsp(uint8_t status, uint16_t len, const void *param,
>                                                         void *user_data)
>  {
> -       const struct mgmt_rp_read_security_info *rp = param;
> +       const struct mgmt_rp_read_controller_cap *rp = param;
>         uint16_t index = PTR_TO_UINT(user_data);
>
>         if (status != 0) {
> @@ -1546,7 +1546,7 @@ static void sec_info_rsp(uint8_t status, uint16_t len, const void *param,
>         }
>
>         print("Primary controller (hci%u)", index);
> -       print("\tSecurity info length: %u", le16_to_cpu(rp->sec_len));
> +       print("\tSecurity info length: %u", le16_to_cpu(rp->cap_len));
>
>  done:
>         pending_index--;
> @@ -1589,11 +1589,11 @@ static void sec_index_rsp(uint8_t status, uint16_t len, const void *param,
>                 if (rp->entry[i].type != 0x00)
>                         continue;
>
> -               if (!mgmt_send(mgmt, MGMT_OP_READ_SECURITY_INFO,
> +               if (!mgmt_send(mgmt, MGMT_OP_READ_CONTROLLER_CAP,
>                                                 index, 0, NULL, sec_info_rsp,
>                                                 UINT_TO_PTR(index), NULL)) {
> -                               error("Unable to send read_security_info cmd");
> -                               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> +                       error("Unable to send read_security_info cmd");
> +                       return bt_shell_noninteractive_quit(EXIT_FAILURE);
>                 }
>                 pending_index++;
>         }
> @@ -1615,7 +1615,7 @@ static void cmd_secinfo(int argc, char **argv)
>                 return;
>         }
>
> -       if (!mgmt_send(mgmt, MGMT_OP_READ_SECURITY_INFO, mgmt_index, 0, NULL,
> +       if (!mgmt_send(mgmt, MGMT_OP_READ_CONTROLLER_CAP, mgmt_index, 0, NULL,
>                                         sec_info_rsp,
>                                         UINT_TO_PTR(mgmt_index), NULL)) {
>                 error("Unable to send read_security_info cmd");
> --
> 2.28.0.709.gb0816b6eb0-goog
>


-- 
Luiz Augusto von Dentz

  reply	other threads:[~2020-10-27 22:08 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-01 23:01 [Bluez PATCH v4 00/10] Bluetooth: Add new MGMT interface for advertising add Daniel Winkler
2020-10-01 23:01 ` [Bluez PATCH v4 01/10] doc/advertising-api: update API with new interface Daniel Winkler
2020-10-27 21:59   ` Luiz Augusto von Dentz
2020-10-01 23:01 ` [Bluez PATCH v4 02/10] doc/mgmt-api: Add new Advertising MGMT interfaces to mgmt-api Daniel Winkler
2020-10-01 23:01 ` [Bluez PATCH v4 03/10] doc/mgmt-api: Update controller capabilities MGMT command in mgmt-api Daniel Winkler
2020-10-01 23:01 ` [Bluez PATCH v4 04/10] advertising: Detect if extended advertising mgmt commands are supported Daniel Winkler
2020-10-01 23:01 ` [Bluez PATCH v4 05/10] advertising: Parse intervals and tx power from adv Daniel Winkler
2020-10-27 22:03   ` Luiz Augusto von Dentz
2020-10-01 23:01 ` [Bluez PATCH v4 06/10] advertising: Use new mgmt interface for advertising add Daniel Winkler
2020-10-01 23:01 ` [Bluez PATCH v4 07/10] advertising: Query LE TX range at manager initialization Daniel Winkler
2020-10-27 22:08   ` Luiz Augusto von Dentz [this message]
2020-10-01 23:01 ` [Bluez PATCH v4 08/10] advertising: Expose SupportedCapabilities for advertising Daniel Winkler
2020-10-27 22:05   ` Luiz Augusto von Dentz
2020-10-01 23:01 ` [Bluez PATCH v4 09/10] client: Add SupportedCapabilities to bluetoothctl Daniel Winkler
2020-10-01 23:01 ` [Bluez PATCH v4 10/10] monitor: Add new MGMT adv commands and events to monitor Daniel Winkler
2020-10-12 15:53 ` [Bluez PATCH v4 00/10] Bluetooth: Add new MGMT interface for advertising add Daniel Winkler
2020-10-26 22:02 ` Daniel Winkler
2020-10-27 22:13   ` Luiz Augusto von Dentz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABBYNZK3SQeWp3OHSgapqL+HEZqcFuPFbwMFCiU9m-vAKd-1gQ@mail.gmail.com \
    --to=luiz.dentz@gmail.com \
    --cc=alainm@chromium.org \
    --cc=chromeos-bluetooth-upstreaming@chromium.org \
    --cc=danielwinkler@google.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=luiz.von.dentz@intel.com \
    --cc=sonnysasaka@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.