Hi Daniel,
On Wed, Sep 16, 2020 at 4:25 PM Daniel Winkler <danielwinkler@xxxxxxxxxx> wrote:
>
> We need to know if kernel supports the new MGMT interface. To do so, we
> call MGMT_OP_READ_COMMANDS when our manager is created and check if the
> new commands are available. This will then be used to route our requests
> for new advertisements.
>
> The change is tested by manually verifying that the correct MGMT
> commands are used when the feature is and is not available in kernel.
>
> Reviewed-by: Sonny Sasaka <sonnysasaka@xxxxxxxxxxxx>
> ---
>
> src/advertising.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/src/advertising.c b/src/advertising.c
> index e5f25948d..172a83907 100644
> --- a/src/advertising.c
> +++ b/src/advertising.c
> @@ -57,6 +57,7 @@ struct btd_adv_manager {
> uint8_t max_ads;
> uint32_t supported_flags;
> unsigned int instance_bitmap;
> + bool extended_add_cmds;
> };
>
> #define AD_TYPE_BROADCAST 0
> @@ -1407,6 +1408,51 @@ static void read_adv_features_callback(uint8_t status, uint16_t length,
> remove_advertising(manager, 0);
> }
>
> +static void read_commands_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_commands *rp = param;
> + uint16_t num_commands, num_events;
> + size_t expected_len;
> + int i;
> +
> + if (status != MGMT_STATUS_SUCCESS) {
> + error("Failed to read supported commands: %s (0x%02x)",
> + mgmt_errstr(status), status);
> + return;
> + }
> +
> + if (length < sizeof(*rp)) {
> + error("Wrong size of read commands response");
> + return;
> + }
> +
> + num_commands = btohs(rp->num_commands);
> + num_events = btohs(rp->num_events);
> +
> + expected_len = sizeof(*rp) + num_commands * sizeof(uint16_t) +
> + num_events * sizeof(uint16_t);
> +
> + if (length < expected_len) {
> + error("Too small reply for supported commands: (%u != %zu)",
> + length, expected_len);
> + return;
> + }
> +
> + for (i = 0; i < num_commands; i++) {
> + uint16_t op = get_le16(rp->opcodes + i);
> +
> + switch (op) {
> + case MGMT_OP_ADD_EXT_ADV_PARAMS:
> + manager->extended_add_cmds = true;
> + break;
> + default:
> + break;
> + }
> + }
> +}
I wouldn't duplicate the handling of MGMT_OP_READ_COMMANDS, so I would
move this to adapter.c and instead use btd_has_kernel_features in
advertising.c
> static struct btd_adv_manager *manager_create(struct btd_adapter *adapter,
> struct mgmt *mgmt)
> {
> @@ -1426,6 +1472,7 @@ static struct btd_adv_manager *manager_create(struct btd_adapter *adapter,
> manager->mgmt_index = btd_adapter_get_index(adapter);
> manager->clients = queue_new();
> manager->supported_flags = MGMT_ADV_FLAG_LOCAL_NAME;
> + manager->extended_add_cmds = false;
>
> if (!g_dbus_register_interface(btd_get_dbus_connection(),
> adapter_get_path(manager->adapter),
> @@ -1442,6 +1489,13 @@ static struct btd_adv_manager *manager_create(struct btd_adapter *adapter,
> goto fail;
> }
>
> + /* Determine if kernel supports extended advertising add command. We
> + * don't care if this request fails, as we will fall back to legacy
> + * add_advertising by default
> + */
> + mgmt_send(manager->mgmt, MGMT_OP_READ_COMMANDS, MGMT_INDEX_NONE, 0,
> + NULL, read_commands_complete, manager, NULL);
> +
> return manager;
>
> fail:
> --
> 2.28.0.618.gf4bc123cb7-goog
>
--
Luiz Augusto von Dentz
