2015-03-20 00:04:31

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 0/6] Introduce APIs to set advertising data

This patch set implements the Add/Remove Advertising commands and the
Advertising Added/Removed events. This provides the bare minimum support
for adding at most one advertising instance, with only support for
setting the advertising data parameter.

Arman Uguray (6):
Bluetooth: Add definitions for Add/Remove Advertising mgmt commands
Bluetooth: Introduce HCI_ADVERTISING_INSTANCE setting and add AD flags
Bluetooth: Add data structure for advertising instance
Bluetooth: Implement the Add Advertising command
Bluetooth: Implement the Remove Advertising command
Bluetooth: Add Advertising Added/Removed events

include/net/bluetooth/hci.h | 14 ++
include/net/bluetooth/hci_core.h | 18 ++
include/net/bluetooth/mgmt.h | 34 ++++
net/bluetooth/hci_core.c | 1 +
net/bluetooth/mgmt.c | 416 ++++++++++++++++++++++++++++++++++++++-
5 files changed, 473 insertions(+), 10 deletions(-)

--
2.2.0.rc0.207.ga3a616c



2015-03-20 00:48:03

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 4/6] Bluetooth: Implement the Add Advertising command

Hi Arman,

> This patch adds the most basic implementation for the
> "Add Advertisement" command. All state updates between the
> various HCI settings (POWERED, ADVERTISING, ADVERTISING_INSTANCE,
> and LE_ENABLED) has been implemented. The command currently
> supports only setting the advertising data fields, with no flags
> and no scan response data.
>
> Signed-off-by: Arman Uguray <[email protected]>
> ---
> net/bluetooth/mgmt.c | 284 ++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 271 insertions(+), 13 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 467cac9..f4ed4fb 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -866,7 +866,7 @@ static u8 get_adv_discov_flags(struct hci_dev *hdev)
> return 0;
> }
>
> -static u8 create_adv_data(struct hci_dev *hdev, u8 *ptr)
> +static u8 create_default_adv_data(struct hci_dev *hdev, u8 *ptr)
> {
> u8 ad_len = 0, flags = 0;
>
> @@ -898,6 +898,35 @@ static u8 create_adv_data(struct hci_dev *hdev, u8 *ptr)
> return ad_len;
> }
>
> +static u8 create_instance_adv_data(struct hci_dev *hdev, u8 *ptr)
> +{
> + u8 ad_len = 0, flags = 0;
> +
> + if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED))
> + flags |= LE_AD_NO_BREDR;
> +
> + /* TODO: Set the appropriate entries based on advertising instance flags
> + * here once flags other than 0 are supported.
> + */
> +
> + if (flags) {
> + BT_DBG("instance adv flags 0x%02x", flags);
> +
> + ptr[0] = 2;
> + ptr[1] = EIR_FLAGS;
> + ptr[2] = flags;
> +
> + ad_len += 3;
> + ptr += 3;
> + }
> +
> + memcpy(ptr, hdev->adv_instance.adv_data,
> + hdev->adv_instance.adv_data_len);
> + ad_len += hdev->adv_instance.adv_data_len;
> +
> + return ad_len;
> +}
> +

for the time being I would keep this separate. I think you are trying to much in a single patch. If you go with initially not supporting any of our flags, then it is pretty much just copy the data over. Done.

Figuring out on how we can combine the two code path to share things can be done in a second step.

> static void update_adv_data(struct hci_request *req)
> {
> struct hci_dev *hdev = req->hdev;
> @@ -909,7 +938,11 @@ static void update_adv_data(struct hci_request *req)
>
> memset(&cp, 0, sizeof(cp));
>
> - len = create_adv_data(hdev, cp.data);
> + if (hci_dev_test_flag(hdev, HCI_ADVERTISING) ||
> + !hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE))
> + len = create_default_adv_data(hdev, cp.data);
> + else
> + len = create_instance_adv_data(hdev, cp.data);

if (HCI_ADVERTISING) {
} else if (HCI_ADVERTISING_INSTANCE) {
}

This seems to be what you are looking for. And this deserves a comment above explaining why things are done this way.

>
> if (hdev->adv_data_len == len &&
> memcmp(cp.data, hdev->adv_data, len) == 0)
> @@ -1692,7 +1725,8 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
> hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, sizeof(scan), &scan);
>
> update_ad:
> - update_adv_data(&req);
> + if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
> + update_adv_data(&req);
>
> err = hci_req_run(&req, set_discoverable_complete);
> if (err < 0)
> @@ -4364,10 +4398,17 @@ static int set_device_id(struct sock *sk, struct hci_dev *hdev, void *data,
> return err;
> }
>
> +static void enable_advertising_instance(struct hci_dev *hdev, u8 status,
> + u16 opcode)
> +{
> + BT_DBG("status %d", status);
> +}
> +
> static void set_advertising_complete(struct hci_dev *hdev, u8 status,
> u16 opcode)
> {
> struct cmd_lookup match = { NULL, hdev };
> + struct hci_request req;
>
> hci_dev_lock(hdev);
>
> @@ -4392,6 +4433,18 @@ static void set_advertising_complete(struct hci_dev *hdev, u8 status,
> if (match.sk)
> sock_put(match.sk);
>
> + /* If instance advertising was set up, then reconfigure advertising */
> + if (!hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE))
> + goto unlock;
> +
> + hci_req_init(&req, hdev);
> +
> + update_adv_data(&req);
> + enable_advertising(&req);
> +
> + if (hci_req_run(&req, enable_advertising_instance) < 0)
> + BT_ERR("Failed to re-configure advertising");
> +

I do not understand this change. Set Advertising take precedence over Add Advertising.

> unlock:
> hci_dev_unlock(hdev);
> }
> @@ -6295,6 +6348,13 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
> hci_dev_lock(hdev);
>
> rp_len = sizeof(*rp);
> +
> + /* Currently only one instance is supported, so just add 1 to the
> + * response length.
> + */
> + if (hdev->cur_adv_index)
> + rp_len++;
> +

I would use the HCI_ADVERTISING_INSTANCE flag here.

> rp = kmalloc(rp_len, GFP_ATOMIC);
> if (!rp) {
> hci_dev_unlock(hdev);
> @@ -6302,10 +6362,16 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
> }
>
> rp->supported_flags = cpu_to_le32(0);
> - rp->max_adv_data_len = 31;
> - rp->max_scan_rsp_len = 31;
> - rp->max_instances = 0;
> - rp->num_instances = 0;
> + rp->max_adv_data_len = HCI_MAX_AD_LENGTH;
> + rp->max_scan_rsp_len = HCI_MAX_AD_LENGTH;
> + rp->max_instances = 1;
> +
> + /* Currently only one instance is supported, so simply return the
> + * current instance number.
> + */
> + rp->num_instances = hdev->cur_adv_index ? 1 : 0;

Same here. Use the flag.

> + if (hdev->cur_adv_index)
> + rp->instance[0] = hdev->cur_adv_index;

The instance id will always be 1 in this case.

>
> hci_dev_unlock(hdev);
>
> @@ -6317,14 +6383,198 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
> return err;
> }
>
> +static bool adv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *adv_data,
> + u8 adv_data_len)
> +{
> + u8 max_adv_len = HCI_MAX_AD_LENGTH;
> + int i, cur_len;
> +
> + /* TODO: Correctly reduce adv_len based on adv_flags. */
> +
> + /* The only global settings that affects the maximum length is BREDR
> + * support, which must be included in the "flags" AD field.
> + */
> + if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED))
> + max_adv_len -= 3;
> +
> + if (adv_data_len > max_adv_len)
> + return false;
> +
> + /* Make sure that adv_data is correctly formatted and doesn't contain
> + * any fields that are managed by adv_flags and global settings.
> + */
> + for (i = 0, cur_len = 0; i < adv_data_len; i += (cur_len + 1)) {
> + cur_len = adv_data[i];
> +
> + /* If the current field length would exceed the total data
> + * length, then it's invalid.
> + */
> + if (i + cur_len >= adv_data_len)
> + return false;
> +
> + /* We use a whitelist for the allowed AD fields */
> + switch (adv_data[i + 1]) {
> + case EIR_UUID16_SOME:
> + case EIR_UUID16_ALL:
> + case EIR_UUID32_SOME:
> + case EIR_UUID32_ALL:
> + case EIR_UUID128_SOME:
> + case EIR_UUID128_ALL:
> + case EIR_NAME_SHORT:
> + case EIR_NAME_COMPLETE:
> + case LE_AD_FIELD_SOLICIT16:
> + case LE_AD_FIELD_SOLICIT32:
> + case LE_AD_FIELD_SOLICIT128:
> + case LE_AD_FIELD_SVC_DATA16:
> + case LE_AD_FIELD_SVC_DATA32:
> + case LE_AD_FIELD_SVC_DATA128:
> + case LE_AD_FIELD_PUB_TRGT_ADDR:
> + case LE_AD_FIELD_RND_TRGT_ADDR:
> + case LE_AD_FIELD_APPEARANCE:
> + case LE_AD_FIELD_MANUF_DATA:
> + break;
> + default:
> + return false;
> + }
> + }
> +
> + return true;
> +}

I would not bother here. If you want to violate the specification, then fine by me. I think the only thing we should check the the TLV structure is actually valid. What fields we expose is not something the kernel should concern itself.

At some point, when we have the extra flags supported, we might want to ensure that these are not included. However including unknown or new types is fine by me.

> +
> +static void add_advertising_complete(struct hci_dev *hdev, u8 status,
> + u16 opcode)
> +{
> + struct mgmt_pending_cmd *cmd;
> + struct mgmt_rp_add_advertising rp;
> +
> + BT_DBG("status %d", status);
> +
> + hci_dev_lock(hdev);
> +
> + if (status) {
> + hci_dev_clear_flag(hdev, HCI_ADVERTISING_INSTANCE);
> + hdev->cur_adv_index = 0;
> +
> + /* TODO: Send Advertising Removed event */

Don't bother with the TODO entries here. You are adding it in the patch series. No need to add a comment for it.

> + }
> +
> + cmd = pending_find(MGMT_OP_ADD_ADVERTISING, hdev);
> + if (!cmd)
> + goto unlock;
> +
> + rp.instance = hdev->cur_adv_index;
> +
> + if (status)
> + mgmt_cmd_status(cmd->sk, cmd->index, cmd->opcode,
> + mgmt_status(status));
> + else
> + mgmt_cmd_complete(cmd->sk, cmd->index, cmd->opcode,
> + mgmt_status(status), &rp, sizeof(rp));
> +
> + mgmt_pending_remove(cmd);
> +
> +unlock:
> + hci_dev_unlock(hdev);
> +}
> +
> static int add_advertising(struct sock *sk, struct hci_dev *hdev,
> void *data, u16 data_len)
> {
> + struct mgmt_cp_add_advertising *cp = data;
> + struct mgmt_rp_add_advertising rp;
> + u32 flags;
> + u8 status;
> + int err;
> + struct mgmt_pending_cmd *cmd;
> + struct hci_request req;
> +
> BT_DBG("%s", hdev->name);
>
> - /* TODO: Implement this command. */
> - return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
> - MGMT_STATUS_NOT_SUPPORTED);
> + status = mgmt_le_support(hdev);
> + if (status)
> + return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
> + status);
> +
> + /* The current implementation supports the bare minimum set of features:
> + * - Single instance.
> + * - 0 flag
> + * - No timeouts
> + * - Only advertising data can be modified.

I think the last two is something we have to have support for. Not in this patch, but in this series before we can merge it. The duration value can be ignored because it is single instance and there it just needs basic validation check and nothing else.

> + * Return error if the parameters contain any unsupported parameters.
> + */
> + if (cp->instance != 1 || cp->flags || cp->timeout || cp->scan_rsp_len)

Check it with 0x01 here. I prefer that. And we might want to check them individually. As I said, the last two need to be supported before we can merge this upstream. So might as well just deal with it.

> + return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
> + MGMT_STATUS_INVALID_PARAMS);
> +
> + flags = __le32_to_cpu(cp->flags);

Do that before you validity checks and not after. You are asking for an endian bug here.

> +
> + hci_dev_lock(hdev);
> +
> + if (pending_find(MGMT_OP_ADD_ADVERTISING, hdev) ||
> + pending_find(MGMT_OP_SET_LE, hdev)) {
> + err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
> + MGMT_STATUS_BUSY);
> + goto unlock;
> + }
> +
> + if (!adv_data_is_valid(hdev, flags, cp->data, cp->adv_data_len)) {
> + err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
> + MGMT_STATUS_INVALID_PARAMS);
> + goto unlock;
> + }
> +
> + hdev->adv_instance.flags = cp->flags;

Endian bug.

> + hdev->adv_instance.adv_data_len = cp->adv_data_len;
> + hdev->adv_instance.scan_rsp_len = cp->scan_rsp_len;
> +
> + if (cp->adv_data_len)
> + memcpy(hdev->adv_instance.adv_data, cp->data, cp->adv_data_len);
> +
> + if (cp->scan_rsp_len)
> + memcpy(hdev->adv_instance.scan_rsp_data,
> + cp->data + cp->adv_data_len, cp->scan_rsp_len);
> +
> + /* TODO: Send Advertising Added event if the index changed
> + * from 0 to 1.
> + */

Skip the TODO comments. Just implement it later on.

> + hdev->cur_adv_index = cp->instance;
> +
> + hci_dev_test_and_set_flag(hdev, HCI_ADVERTISING_INSTANCE);

hdev_dev_set_flag() is the same here. You need to do something with the value your are testing for or it is pointless.

> +
> + /* If the HCI_ADVERTISING flag is set or the device isn't powered then
> + * we have no HCI communication to make. Simply return.
> + */
> + if (!hdev_is_powered(hdev) ||
> + hci_dev_test_flag(hdev, HCI_ADVERTISING)) {
> + rp.instance = hdev->cur_adv_index;
> + err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
> + MGMT_STATUS_SUCCESS, &rp, sizeof(rp));
> + goto unlock;
> + }
> +
> + /* We're good to go, update advertising data, parameters, and start
> + * advertising.
> + */
> + cmd = mgmt_pending_add(sk, MGMT_OP_ADD_ADVERTISING, hdev, data,
> + data_len);
> + if (!cmd) {
> + err = -ENOMEM;
> + goto unlock;
> + }
> +
> + hci_req_init(&req, hdev);
> +
> + update_adv_data(&req);
> + enable_advertising(&req);
> +
> + err = hci_req_run(&req, add_advertising_complete);
> + if (err < 0)
> + mgmt_pending_remove(cmd);
> +
> +unlock:
> + hci_dev_unlock(hdev);
> +
> + return err;
> }
>
> static int remove_advertising(struct sock *sk, struct hci_dev *hdev,
> @@ -6595,7 +6845,8 @@ static int powered_update_hci(struct hci_dev *hdev)
> update_scan_rsp_data(&req);
> }
>
> - if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
> + if (hci_dev_test_flag(hdev, HCI_ADVERTISING) ||
> + hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE))
> enable_advertising(&req);
>
> restart_le_actions(&req);
> @@ -6707,7 +6958,13 @@ void mgmt_discoverable_timeout(struct hci_dev *hdev)
> sizeof(scan), &scan);
> }
> update_class(&req);
> - update_adv_data(&req);
> +
> + /* Advertising instances don't use the global discoverable setting, so
> + * only update AD if advertising was enabled using Set Advertising.
> + */
> + if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
> + update_adv_data(&req);
> +
> hci_req_run(&req, NULL);
>
> hdev->discov_timeout = 0;
> @@ -7608,7 +7865,8 @@ void mgmt_reenable_advertising(struct hci_dev *hdev)
> {
> struct hci_request req;
>
> - if (!hci_dev_test_flag(hdev, HCI_ADVERTISING))
> + if (!hci_dev_test_flag(hdev, HCI_ADVERTISING) &&
> + !hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE))
> return;
>
> hci_req_init(&req, hdev);

Regards

Marcel


2015-03-20 00:47:56

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 6/6] Bluetooth: Add Advertising Added/Removed events

Hi Arman,

> This patch introduces the Advertising Added and Advertising Removed
> events. The events are sent when an advertising instance is added
> or removed from an hci_dev.
>
> Signed-off-by: Arman Uguray <[email protected]>
> ---
> include/net/bluetooth/mgmt.h | 10 ++++++++++
> net/bluetooth/mgmt.c | 37 +++++++++++++++++++++++++++++--------
> 2 files changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index c60a408..874e2a2 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -766,3 +766,13 @@ struct mgmt_ev_local_oob_data_updated {
> __le16 eir_len;
> __u8 eir[0];
> } __packed;
> +
> +#define MGMT_EV_ADVERTISING_ADDED 0x0023
> +struct mgmt_ev_advertising_added {
> + __u8 instance;
> +} __packed;
> +
> +#define MGMT_EV_ADVERTISING_REMOVED 0x0024
> +struct mgmt_ev_advertising_removed {
> + __u8 instance;
> +} __packed;

put these into your first patch. I am pretty confident that these will not change much.

> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index eae280d..5d04dd8 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -137,6 +137,8 @@ static const u16 mgmt_events[] = {
> MGMT_EV_EXT_INDEX_ADDED,
> MGMT_EV_EXT_INDEX_REMOVED,
> MGMT_EV_LOCAL_OOB_DATA_UPDATED,
> + MGMT_EV_ADVERTISING_ADDED,
> + MGMT_EV_ADVERTISING_REMOVED,
> };
>
> #define CACHE_TIMEOUT msecs_to_jiffies(2 * 1000)
> @@ -6441,6 +6443,24 @@ static bool adv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *adv_data,
> return true;
> }
>
> +static void advertising_added(struct hci_dev *hdev, u8 instance)
> +{
> + struct mgmt_ev_advertising_added ev;
> +
> + ev.instance = instance;
> +
> + mgmt_event(MGMT_EV_ADVERTISING_ADDED, hdev, &ev, sizeof(ev), NULL);
> +}
> +
> +static void advertising_removed(struct hci_dev *hdev, u8 instance)
> +{
> + struct mgmt_ev_advertising_removed ev;
> +
> + ev.instance = instance;
> +
> + mgmt_event(MGMT_EV_ADVERTISING_REMOVED, hdev, &ev, sizeof(ev), NULL);
> +}
> +

You want to skip the socket the Add Advertising command has been executed on. Like we do with every other command. So you need to support that. Look at device_added().

> static void add_advertising_complete(struct hci_dev *hdev, u8 status,
> u16 opcode)
> {
> @@ -6453,9 +6473,9 @@ static void add_advertising_complete(struct hci_dev *hdev, u8 status,
>
> if (status) {
> hci_dev_clear_flag(hdev, HCI_ADVERTISING_INSTANCE);
> - hdev->cur_adv_index = 0;
> + advertising_removed(hdev, hdev->cur_adv_index);
>
> - /* TODO: Send Advertising Removed event */
> + hdev->cur_adv_index = 0;
> }
>
> cmd = pending_find(MGMT_OP_ADD_ADVERTISING, hdev);
> @@ -6535,10 +6555,11 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
> memcpy(hdev->adv_instance.scan_rsp_data,
> cp->data + cp->adv_data_len, cp->scan_rsp_len);
>
> - /* TODO: Send Advertising Added event if the index changed
> - * from 0 to 1.
> - */
> - hdev->cur_adv_index = cp->instance;
> + if (hdev->cur_adv_index != cp->instance) {
> + hdev->cur_adv_index = cp->instance;
> +
> + advertising_added(hdev, hdev->cur_adv_index);
> + }
>
> hci_dev_set_flag(hdev, HCI_ADVERTISING_INSTANCE);
>
> @@ -6640,11 +6661,11 @@ static int remove_advertising(struct sock *sk, struct hci_dev *hdev,
> goto unlock;
> }
>
> + advertising_removed(hdev, hdev->cur_adv_index);
> +
> memset(&hdev->adv_instance, 0, sizeof(hdev->adv_instance));
> hdev->cur_adv_index = 0;
>
> - /* TODO" Send Advertising Removed event */
> -
> hci_dev_clear_flag(hdev, HCI_ADVERTISING_INSTANCE);
>
> /* If the HCI_ADVERTISING flag is set or the device isn't powered then

since this is all pretty much simple, you might just squash this into the patches for the add and remove feature. Seems pretty straight forward from an event perspective.

Regards

Marcel


2015-03-20 00:47:55

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 3/6] Bluetooth: Add data structure for advertising instance

Hi Arman,

> This patch introduces a new data structure to represent advertising
> instances that were added using the "Add Advertising" mgmt command.
> Initially an hci_dev structure will support only one of these instances
> at a time, so the current instance is simply stored as a direct member
> of hci_dev.
>
> Signed-off-by: Arman Uguray <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 18 ++++++++++++++++++
> net/bluetooth/hci_core.c | 1 +
> 2 files changed, 19 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index b65c53d..4e32e06 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -155,6 +155,15 @@ struct oob_data {
> u8 rand256[16];
> };
>
> +struct le_adv_instance {
> + __u8 index;

I would prefer that we better call it id or instance here. I also wonder if we should just have this as le_adv_info struct to keep the names a bit shorter. And in that case then use rather __u8 instance over __u8 id.

You also do not need to carry the le_ prefix around all the time. Advertising is by definition LE only. I would just strip it here.

> + __u32 flags;
> + __u16 adv_data_len;
> + __u8 adv_data[HCI_MAX_AD_LENGTH];
> + __u16 scan_rsp_len;
> + __u8 scan_rsp_data[HCI_MAX_AD_LENGTH];
> +};
> +
> #define HCI_MAX_SHORT_NAME_LENGTH 10
>
> /* Default LE RPA expiry time, 15 minutes */
> @@ -364,6 +373,9 @@ struct hci_dev {
> __u8 scan_rsp_data[HCI_MAX_AD_LENGTH];
> __u8 scan_rsp_data_len;
>
> + struct le_adv_instance adv_instance;
> + __u8 cur_adv_index;
> +

I do not follow what the cur_adv_index would give us. Since initially we would only support a single instance, we can just use the hdev->dev_flag for this that you defined.

> __u8 irk[16];
> __u32 rpa_timeout;
> struct delayed_work rpa_expired;
> @@ -550,6 +562,12 @@ static inline void hci_discovery_filter_clear(struct hci_dev *hdev)
> hdev->discovery.scan_duration = 0;
> }
>
> +static inline void hci_le_adv_instance_init(struct hci_dev *hdev)
> +{
> + memset(&hdev->adv_instance, 0, sizeof(struct le_adv_instance));
> + hdev->cur_adv_index = 0;
> +}
> +
> bool hci_discovery_active(struct hci_dev *hdev);
>
> void hci_discovery_set_state(struct hci_dev *hdev, int state);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 773f216..50a5f4c 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3125,6 +3125,7 @@ struct hci_dev *hci_alloc_dev(void)
>
> hci_init_sysfs(hdev);
> discovery_init(hdev);
> + hci_le_adv_instance_init(hdev);

I would not go overboard with the naming for local functions. So adv_info_init() might be just enough.

Regards

Marcel


2015-03-20 00:47:54

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/6] Bluetooth: Introduce HCI_ADVERTISING_INSTANCE setting and add AD flags

Hi Arman,

> This patch introduces the HCI_ADVERTISING_INSTANCE setting, which is set
> when an at least one advertising instance has been added using the
> "Add Advertising" mgmt command. This patch also adds macro definitions
> various advertising data fields which can be assigned using the
> "Add Advertising" command.
>
> Signed-off-by: Arman Uguray <[email protected]>
> ---
> include/net/bluetooth/hci.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 06e7eee..e400175 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -227,6 +227,7 @@ enum {
> HCI_LE_ENABLED,
> HCI_ADVERTISING,
> HCI_ADVERTISING_CONNECTABLE,
> + HCI_ADVERTISING_INSTANCE,
> HCI_CONNECTABLE,
> HCI_DISCOVERABLE,
> HCI_LIMITED_DISCOVERABLE,
> @@ -477,6 +478,19 @@ enum {
> #define LE_AD_SIM_LE_BREDR_CTRL 0x08 /* Simultaneous LE & BR/EDR Controller */
> #define LE_AD_SIM_LE_BREDR_HOST 0x10 /* Simultaneous LE & BR/EDR Host */
>
> +/* Other Low Energy Advertising Data fielt types */
> +#define LE_AD_FIELD_SOLICIT16 0x14 /* 16-bit service solic. UUIDs */
> +#define LE_AD_FIELD_SOLICIT32 0x1F /* 32-bit service solic. UUIDs */
> +#define LE_AD_FIELD_SOLICIT128 0x15 /* 128-bit service solic. UUIDs */
> +#define LE_AD_FIELD_SVC_DATA16 0x16 /* Service Data - 16-bit UUID */
> +#define LE_AD_FIELD_SVC_DATA32 0x20 /* Service Data - 32-bit UUID */
> +#define LE_AD_FIELD_SVC_DATA128 0x21 /* Service Data - 128-bit UUID */
> +#define LE_AD_FIELD_PUB_TRGT_ADDR 0x17 /* Public Target Address */
> +#define LE_AD_FIELD_RND_TRGT_ADDR 0x18 /* Random Target Address */
> +#define LE_AD_FIELD_APPEARANCE 0x19 /* Appearance */
> +#define LE_AD_FIELD_MANUF_DATA 0xFF /* Manufacturer Specific Data */

please use the EIR_ section for this and extend it. I know this will look odd initially, but since AD and EIR share the same types we actually opted for calling most things EIR and kept it like that.

Also I think the only one you need to add is the APPEARANCE one. All others will not be used inside the kernel and have to be provided by userspace in raw format. So leave them out.

Regards

Marcel


2015-03-20 00:47:52

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/6] Bluetooth: Add definitions for Add/Remove Advertising mgmt commands

Hi Arman,

> This patch adds definitions for the Add Advertising and Remove
> Advertising MGMT commands with stubs that initially return
> "Not Supported" as command status.
>
> Signed-off-by: Arman Uguray <[email protected]>
> ---
> include/net/bluetooth/mgmt.h | 24 ++++++++++++++++++++++++
> net/bluetooth/mgmt.c | 25 +++++++++++++++++++++++++
> 2 files changed, 49 insertions(+)
>
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index a1a6867..c60a408 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -539,6 +539,30 @@ struct mgmt_rp_read_adv_features {
> __u8 instance[0];
> } __packed;
>
> +#define MGMT_OP_ADD_ADVERTISING 0x003E
> +struct mgmt_cp_add_advertising {
> + __u8 instance;
> + __le32 flags;
> + __le16 duration;
> + __le16 timeout;
> + __u8 adv_data_len;
> + __u8 scan_rsp_len;
> + __u8 data[0];
> +} __packed;
> +#define MGMT_ADD_ADVERTISING_SIZE 11
> +struct mgmt_rp_add_advertising {
> + __u8 instance;
> +} __packed;
> +
> +#define MGMT_OP_REMOVE_ADVERTISING 0x003F
> +struct mgmt_cp_remove_advertising {
> + __u8 instance;
> +} __packed;
> +#define MGMT_REMOVE_ADVERTISING_SIZE 1
> +struct mgmt_rp_remove_advertising {
> + __u8 instance;
> +} __packed;
> +

this looks good. I would just include the event data structs and constants here as well. They can come as a single patch.

> #define MGMT_EV_CMD_COMPLETE 0x0001
> struct mgmt_ev_cmd_complete {
> __le16 opcode;
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index f3a9579..467cac9 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -100,6 +100,8 @@ static const u16 mgmt_commands[] = {
> MGMT_OP_READ_LOCAL_OOB_EXT_DATA,
> MGMT_OP_READ_EXT_INDEX_LIST,
> MGMT_OP_READ_ADV_FEATURES,
> + MGMT_OP_ADD_ADVERTISING,
> + MGMT_OP_REMOVE_ADVERTISING,
> };
>
> static const u16 mgmt_events[] = {
> @@ -6315,6 +6317,26 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
> return err;
> }
>
> +static int add_advertising(struct sock *sk, struct hci_dev *hdev,
> + void *data, u16 data_len)
> +{
> + BT_DBG("%s", hdev->name);
> +
> + /* TODO: Implement this command. */
> + return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
> + MGMT_STATUS_NOT_SUPPORTED);
> +}
> +
> +static int remove_advertising(struct sock *sk, struct hci_dev *hdev,
> + void *data, u16 data_len)
> +{
> + BT_DBG("%s", hdev->name);
> +
> + /* TODO: Implement this command. */
> + return mgmt_cmd_status(sk, hdev->id, MGMT_OP_REMOVE_ADVERTISING,
> + MGMT_STATUS_NOT_SUPPORTED);
> +}

Lets not do this. Just add the structs and constants and then add the implementation in the next patch. Doing it with an empty stub just creates noise in the patchset and we have not done it that way in the past. At least not for kernel code.

Regards

Marcel


2015-03-20 00:04:37

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 6/6] Bluetooth: Add Advertising Added/Removed events

This patch introduces the Advertising Added and Advertising Removed
events. The events are sent when an advertising instance is added
or removed from an hci_dev.

Signed-off-by: Arman Uguray <[email protected]>
---
include/net/bluetooth/mgmt.h | 10 ++++++++++
net/bluetooth/mgmt.c | 37 +++++++++++++++++++++++++++++--------
2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index c60a408..874e2a2 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -766,3 +766,13 @@ struct mgmt_ev_local_oob_data_updated {
__le16 eir_len;
__u8 eir[0];
} __packed;
+
+#define MGMT_EV_ADVERTISING_ADDED 0x0023
+struct mgmt_ev_advertising_added {
+ __u8 instance;
+} __packed;
+
+#define MGMT_EV_ADVERTISING_REMOVED 0x0024
+struct mgmt_ev_advertising_removed {
+ __u8 instance;
+} __packed;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index eae280d..5d04dd8 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -137,6 +137,8 @@ static const u16 mgmt_events[] = {
MGMT_EV_EXT_INDEX_ADDED,
MGMT_EV_EXT_INDEX_REMOVED,
MGMT_EV_LOCAL_OOB_DATA_UPDATED,
+ MGMT_EV_ADVERTISING_ADDED,
+ MGMT_EV_ADVERTISING_REMOVED,
};

#define CACHE_TIMEOUT msecs_to_jiffies(2 * 1000)
@@ -6441,6 +6443,24 @@ static bool adv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *adv_data,
return true;
}

+static void advertising_added(struct hci_dev *hdev, u8 instance)
+{
+ struct mgmt_ev_advertising_added ev;
+
+ ev.instance = instance;
+
+ mgmt_event(MGMT_EV_ADVERTISING_ADDED, hdev, &ev, sizeof(ev), NULL);
+}
+
+static void advertising_removed(struct hci_dev *hdev, u8 instance)
+{
+ struct mgmt_ev_advertising_removed ev;
+
+ ev.instance = instance;
+
+ mgmt_event(MGMT_EV_ADVERTISING_REMOVED, hdev, &ev, sizeof(ev), NULL);
+}
+
static void add_advertising_complete(struct hci_dev *hdev, u8 status,
u16 opcode)
{
@@ -6453,9 +6473,9 @@ static void add_advertising_complete(struct hci_dev *hdev, u8 status,

if (status) {
hci_dev_clear_flag(hdev, HCI_ADVERTISING_INSTANCE);
- hdev->cur_adv_index = 0;
+ advertising_removed(hdev, hdev->cur_adv_index);

- /* TODO: Send Advertising Removed event */
+ hdev->cur_adv_index = 0;
}

cmd = pending_find(MGMT_OP_ADD_ADVERTISING, hdev);
@@ -6535,10 +6555,11 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
memcpy(hdev->adv_instance.scan_rsp_data,
cp->data + cp->adv_data_len, cp->scan_rsp_len);

- /* TODO: Send Advertising Added event if the index changed
- * from 0 to 1.
- */
- hdev->cur_adv_index = cp->instance;
+ if (hdev->cur_adv_index != cp->instance) {
+ hdev->cur_adv_index = cp->instance;
+
+ advertising_added(hdev, hdev->cur_adv_index);
+ }

hci_dev_set_flag(hdev, HCI_ADVERTISING_INSTANCE);

@@ -6640,11 +6661,11 @@ static int remove_advertising(struct sock *sk, struct hci_dev *hdev,
goto unlock;
}

+ advertising_removed(hdev, hdev->cur_adv_index);
+
memset(&hdev->adv_instance, 0, sizeof(hdev->adv_instance));
hdev->cur_adv_index = 0;

- /* TODO" Send Advertising Removed event */
-
hci_dev_clear_flag(hdev, HCI_ADVERTISING_INSTANCE);

/* If the HCI_ADVERTISING flag is set or the device isn't powered then
--
2.2.0.rc0.207.ga3a616c


2015-03-20 00:04:36

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 5/6] Bluetooth: Implement the Remove Advertising command

This patch implements the "Remove Advertising" mgmt command.

Signed-off-by: Arman Uguray <[email protected]>
---
net/bluetooth/mgmt.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 96 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index f4ed4fb..eae280d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6511,6 +6511,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
hci_dev_lock(hdev);

if (pending_find(MGMT_OP_ADD_ADVERTISING, hdev) ||
+ pending_find(MGMT_OP_REMOVE_ADVERTISING, hdev) ||
pending_find(MGMT_OP_SET_LE, hdev)) {
err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
MGMT_STATUS_BUSY);
@@ -6539,7 +6540,7 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
*/
hdev->cur_adv_index = cp->instance;

- hci_dev_test_and_set_flag(hdev, HCI_ADVERTISING_INSTANCE);
+ hci_dev_set_flag(hdev, HCI_ADVERTISING_INSTANCE);

/* If the HCI_ADVERTISING flag is set or the device isn't powered then
* we have no HCI communication to make. Simply return.
@@ -6577,14 +6578,105 @@ unlock:
return err;
}

+static void remove_advertising_complete(struct hci_dev *hdev, u8 status,
+ u16 opcode)
+{
+ struct mgmt_pending_cmd *cmd;
+ struct mgmt_rp_remove_advertising rp;
+
+ BT_DBG("status %d", status);
+
+ hci_dev_lock(hdev);
+
+ /* A failure status here only means that we failed to disable
+ * advertising. Otherwise, the advertising instance has been removed,
+ * so report success.
+ */
+ cmd = pending_find(MGMT_OP_REMOVE_ADVERTISING, hdev);
+ if (!cmd)
+ goto unlock;
+
+ rp.instance = 1;
+
+ mgmt_cmd_complete(cmd->sk, cmd->index, cmd->opcode, MGMT_STATUS_SUCCESS,
+ &rp, sizeof(rp));
+ mgmt_pending_remove(cmd);
+
+unlock:
+ hci_dev_unlock(hdev);
+}
+
static int remove_advertising(struct sock *sk, struct hci_dev *hdev,
void *data, u16 data_len)
{
+ struct mgmt_cp_remove_advertising *cp = data;
+ struct mgmt_rp_remove_advertising rp;
+ int err;
+ struct mgmt_pending_cmd *cmd;
+ struct hci_request req;
+
BT_DBG("%s", hdev->name);

- /* TODO: Implement this command. */
- return mgmt_cmd_status(sk, hdev->id, MGMT_OP_REMOVE_ADVERTISING,
- MGMT_STATUS_NOT_SUPPORTED);
+ /* The current implementation only allows modifying instance no 1. A
+ * value of 0 indicates that all instances should be cleared.
+ */
+ if (cp->instance > 1)
+ return mgmt_cmd_status(sk, hdev->id, MGMT_OP_REMOVE_ADVERTISING,
+ MGMT_STATUS_INVALID_PARAMS);
+
+ hci_dev_lock(hdev);
+
+ if (pending_find(MGMT_OP_ADD_ADVERTISING, hdev) ||
+ pending_find(MGMT_OP_REMOVE_ADVERTISING, hdev) ||
+ pending_find(MGMT_OP_SET_LE, hdev)) {
+ err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_REMOVE_ADVERTISING,
+ MGMT_STATUS_BUSY);
+ goto unlock;
+ }
+
+ if (!hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE)) {
+ err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_REMOVE_ADVERTISING,
+ MGMT_STATUS_INVALID_PARAMS);
+ goto unlock;
+ }
+
+ memset(&hdev->adv_instance, 0, sizeof(hdev->adv_instance));
+ hdev->cur_adv_index = 0;
+
+ /* TODO" Send Advertising Removed event */
+
+ hci_dev_clear_flag(hdev, HCI_ADVERTISING_INSTANCE);
+
+ /* If the HCI_ADVERTISING flag is set or the device isn't powered then
+ * we have no HCI communication to make. Simply return.
+ */
+ if (!hdev_is_powered(hdev) ||
+ hci_dev_test_flag(hdev, HCI_ADVERTISING)) {
+ rp.instance = 1;
+ err = mgmt_cmd_complete(sk, hdev->id,
+ MGMT_OP_REMOVE_ADVERTISING,
+ MGMT_STATUS_SUCCESS, &rp, sizeof(rp));
+ goto unlock;
+ }
+
+ cmd = mgmt_pending_add(sk, MGMT_OP_REMOVE_ADVERTISING, hdev, data,
+ data_len);
+ if (!cmd) {
+ err = -ENOMEM;
+ goto unlock;
+ }
+
+ hci_req_init(&req, hdev);
+ disable_advertising(&req);
+
+ err = hci_req_run(&req, remove_advertising_complete);
+ if (err < 0)
+ mgmt_pending_remove(cmd);
+
+unlock:
+ hci_dev_unlock(hdev);
+
+ return err;
}

static const struct hci_mgmt_handler mgmt_handlers[] = {
--
2.2.0.rc0.207.ga3a616c


2015-03-20 00:04:35

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 4/6] Bluetooth: Implement the Add Advertising command

This patch adds the most basic implementation for the
"Add Advertisement" command. All state updates between the
various HCI settings (POWERED, ADVERTISING, ADVERTISING_INSTANCE,
and LE_ENABLED) has been implemented. The command currently
supports only setting the advertising data fields, with no flags
and no scan response data.

Signed-off-by: Arman Uguray <[email protected]>
---
net/bluetooth/mgmt.c | 284 ++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 271 insertions(+), 13 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 467cac9..f4ed4fb 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -866,7 +866,7 @@ static u8 get_adv_discov_flags(struct hci_dev *hdev)
return 0;
}

-static u8 create_adv_data(struct hci_dev *hdev, u8 *ptr)
+static u8 create_default_adv_data(struct hci_dev *hdev, u8 *ptr)
{
u8 ad_len = 0, flags = 0;

@@ -898,6 +898,35 @@ static u8 create_adv_data(struct hci_dev *hdev, u8 *ptr)
return ad_len;
}

+static u8 create_instance_adv_data(struct hci_dev *hdev, u8 *ptr)
+{
+ u8 ad_len = 0, flags = 0;
+
+ if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED))
+ flags |= LE_AD_NO_BREDR;
+
+ /* TODO: Set the appropriate entries based on advertising instance flags
+ * here once flags other than 0 are supported.
+ */
+
+ if (flags) {
+ BT_DBG("instance adv flags 0x%02x", flags);
+
+ ptr[0] = 2;
+ ptr[1] = EIR_FLAGS;
+ ptr[2] = flags;
+
+ ad_len += 3;
+ ptr += 3;
+ }
+
+ memcpy(ptr, hdev->adv_instance.adv_data,
+ hdev->adv_instance.adv_data_len);
+ ad_len += hdev->adv_instance.adv_data_len;
+
+ return ad_len;
+}
+
static void update_adv_data(struct hci_request *req)
{
struct hci_dev *hdev = req->hdev;
@@ -909,7 +938,11 @@ static void update_adv_data(struct hci_request *req)

memset(&cp, 0, sizeof(cp));

- len = create_adv_data(hdev, cp.data);
+ if (hci_dev_test_flag(hdev, HCI_ADVERTISING) ||
+ !hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE))
+ len = create_default_adv_data(hdev, cp.data);
+ else
+ len = create_instance_adv_data(hdev, cp.data);

if (hdev->adv_data_len == len &&
memcmp(cp.data, hdev->adv_data, len) == 0)
@@ -1692,7 +1725,8 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, sizeof(scan), &scan);

update_ad:
- update_adv_data(&req);
+ if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
+ update_adv_data(&req);

err = hci_req_run(&req, set_discoverable_complete);
if (err < 0)
@@ -4364,10 +4398,17 @@ static int set_device_id(struct sock *sk, struct hci_dev *hdev, void *data,
return err;
}

+static void enable_advertising_instance(struct hci_dev *hdev, u8 status,
+ u16 opcode)
+{
+ BT_DBG("status %d", status);
+}
+
static void set_advertising_complete(struct hci_dev *hdev, u8 status,
u16 opcode)
{
struct cmd_lookup match = { NULL, hdev };
+ struct hci_request req;

hci_dev_lock(hdev);

@@ -4392,6 +4433,18 @@ static void set_advertising_complete(struct hci_dev *hdev, u8 status,
if (match.sk)
sock_put(match.sk);

+ /* If instance advertising was set up, then reconfigure advertising */
+ if (!hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE))
+ goto unlock;
+
+ hci_req_init(&req, hdev);
+
+ update_adv_data(&req);
+ enable_advertising(&req);
+
+ if (hci_req_run(&req, enable_advertising_instance) < 0)
+ BT_ERR("Failed to re-configure advertising");
+
unlock:
hci_dev_unlock(hdev);
}
@@ -6295,6 +6348,13 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
hci_dev_lock(hdev);

rp_len = sizeof(*rp);
+
+ /* Currently only one instance is supported, so just add 1 to the
+ * response length.
+ */
+ if (hdev->cur_adv_index)
+ rp_len++;
+
rp = kmalloc(rp_len, GFP_ATOMIC);
if (!rp) {
hci_dev_unlock(hdev);
@@ -6302,10 +6362,16 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
}

rp->supported_flags = cpu_to_le32(0);
- rp->max_adv_data_len = 31;
- rp->max_scan_rsp_len = 31;
- rp->max_instances = 0;
- rp->num_instances = 0;
+ rp->max_adv_data_len = HCI_MAX_AD_LENGTH;
+ rp->max_scan_rsp_len = HCI_MAX_AD_LENGTH;
+ rp->max_instances = 1;
+
+ /* Currently only one instance is supported, so simply return the
+ * current instance number.
+ */
+ rp->num_instances = hdev->cur_adv_index ? 1 : 0;
+ if (hdev->cur_adv_index)
+ rp->instance[0] = hdev->cur_adv_index;

hci_dev_unlock(hdev);

@@ -6317,14 +6383,198 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
return err;
}

+static bool adv_data_is_valid(struct hci_dev *hdev, u32 adv_flags, u8 *adv_data,
+ u8 adv_data_len)
+{
+ u8 max_adv_len = HCI_MAX_AD_LENGTH;
+ int i, cur_len;
+
+ /* TODO: Correctly reduce adv_len based on adv_flags. */
+
+ /* The only global settings that affects the maximum length is BREDR
+ * support, which must be included in the "flags" AD field.
+ */
+ if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED))
+ max_adv_len -= 3;
+
+ if (adv_data_len > max_adv_len)
+ return false;
+
+ /* Make sure that adv_data is correctly formatted and doesn't contain
+ * any fields that are managed by adv_flags and global settings.
+ */
+ for (i = 0, cur_len = 0; i < adv_data_len; i += (cur_len + 1)) {
+ cur_len = adv_data[i];
+
+ /* If the current field length would exceed the total data
+ * length, then it's invalid.
+ */
+ if (i + cur_len >= adv_data_len)
+ return false;
+
+ /* We use a whitelist for the allowed AD fields */
+ switch (adv_data[i + 1]) {
+ case EIR_UUID16_SOME:
+ case EIR_UUID16_ALL:
+ case EIR_UUID32_SOME:
+ case EIR_UUID32_ALL:
+ case EIR_UUID128_SOME:
+ case EIR_UUID128_ALL:
+ case EIR_NAME_SHORT:
+ case EIR_NAME_COMPLETE:
+ case LE_AD_FIELD_SOLICIT16:
+ case LE_AD_FIELD_SOLICIT32:
+ case LE_AD_FIELD_SOLICIT128:
+ case LE_AD_FIELD_SVC_DATA16:
+ case LE_AD_FIELD_SVC_DATA32:
+ case LE_AD_FIELD_SVC_DATA128:
+ case LE_AD_FIELD_PUB_TRGT_ADDR:
+ case LE_AD_FIELD_RND_TRGT_ADDR:
+ case LE_AD_FIELD_APPEARANCE:
+ case LE_AD_FIELD_MANUF_DATA:
+ break;
+ default:
+ return false;
+ }
+ }
+
+ return true;
+}
+
+static void add_advertising_complete(struct hci_dev *hdev, u8 status,
+ u16 opcode)
+{
+ struct mgmt_pending_cmd *cmd;
+ struct mgmt_rp_add_advertising rp;
+
+ BT_DBG("status %d", status);
+
+ hci_dev_lock(hdev);
+
+ if (status) {
+ hci_dev_clear_flag(hdev, HCI_ADVERTISING_INSTANCE);
+ hdev->cur_adv_index = 0;
+
+ /* TODO: Send Advertising Removed event */
+ }
+
+ cmd = pending_find(MGMT_OP_ADD_ADVERTISING, hdev);
+ if (!cmd)
+ goto unlock;
+
+ rp.instance = hdev->cur_adv_index;
+
+ if (status)
+ mgmt_cmd_status(cmd->sk, cmd->index, cmd->opcode,
+ mgmt_status(status));
+ else
+ mgmt_cmd_complete(cmd->sk, cmd->index, cmd->opcode,
+ mgmt_status(status), &rp, sizeof(rp));
+
+ mgmt_pending_remove(cmd);
+
+unlock:
+ hci_dev_unlock(hdev);
+}
+
static int add_advertising(struct sock *sk, struct hci_dev *hdev,
void *data, u16 data_len)
{
+ struct mgmt_cp_add_advertising *cp = data;
+ struct mgmt_rp_add_advertising rp;
+ u32 flags;
+ u8 status;
+ int err;
+ struct mgmt_pending_cmd *cmd;
+ struct hci_request req;
+
BT_DBG("%s", hdev->name);

- /* TODO: Implement this command. */
- return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
- MGMT_STATUS_NOT_SUPPORTED);
+ status = mgmt_le_support(hdev);
+ if (status)
+ return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
+ status);
+
+ /* The current implementation supports the bare minimum set of features:
+ * - Single instance.
+ * - 0 flag
+ * - No timeouts
+ * - Only advertising data can be modified.
+ * Return error if the parameters contain any unsupported parameters.
+ */
+ if (cp->instance != 1 || cp->flags || cp->timeout || cp->scan_rsp_len)
+ return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
+ MGMT_STATUS_INVALID_PARAMS);
+
+ flags = __le32_to_cpu(cp->flags);
+
+ hci_dev_lock(hdev);
+
+ if (pending_find(MGMT_OP_ADD_ADVERTISING, hdev) ||
+ pending_find(MGMT_OP_SET_LE, hdev)) {
+ err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
+ MGMT_STATUS_BUSY);
+ goto unlock;
+ }
+
+ if (!adv_data_is_valid(hdev, flags, cp->data, cp->adv_data_len)) {
+ err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
+ MGMT_STATUS_INVALID_PARAMS);
+ goto unlock;
+ }
+
+ hdev->adv_instance.flags = cp->flags;
+ hdev->adv_instance.adv_data_len = cp->adv_data_len;
+ hdev->adv_instance.scan_rsp_len = cp->scan_rsp_len;
+
+ if (cp->adv_data_len)
+ memcpy(hdev->adv_instance.adv_data, cp->data, cp->adv_data_len);
+
+ if (cp->scan_rsp_len)
+ memcpy(hdev->adv_instance.scan_rsp_data,
+ cp->data + cp->adv_data_len, cp->scan_rsp_len);
+
+ /* TODO: Send Advertising Added event if the index changed
+ * from 0 to 1.
+ */
+ hdev->cur_adv_index = cp->instance;
+
+ hci_dev_test_and_set_flag(hdev, HCI_ADVERTISING_INSTANCE);
+
+ /* If the HCI_ADVERTISING flag is set or the device isn't powered then
+ * we have no HCI communication to make. Simply return.
+ */
+ if (!hdev_is_powered(hdev) ||
+ hci_dev_test_flag(hdev, HCI_ADVERTISING)) {
+ rp.instance = hdev->cur_adv_index;
+ err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
+ MGMT_STATUS_SUCCESS, &rp, sizeof(rp));
+ goto unlock;
+ }
+
+ /* We're good to go, update advertising data, parameters, and start
+ * advertising.
+ */
+ cmd = mgmt_pending_add(sk, MGMT_OP_ADD_ADVERTISING, hdev, data,
+ data_len);
+ if (!cmd) {
+ err = -ENOMEM;
+ goto unlock;
+ }
+
+ hci_req_init(&req, hdev);
+
+ update_adv_data(&req);
+ enable_advertising(&req);
+
+ err = hci_req_run(&req, add_advertising_complete);
+ if (err < 0)
+ mgmt_pending_remove(cmd);
+
+unlock:
+ hci_dev_unlock(hdev);
+
+ return err;
}

static int remove_advertising(struct sock *sk, struct hci_dev *hdev,
@@ -6595,7 +6845,8 @@ static int powered_update_hci(struct hci_dev *hdev)
update_scan_rsp_data(&req);
}

- if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
+ if (hci_dev_test_flag(hdev, HCI_ADVERTISING) ||
+ hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE))
enable_advertising(&req);

restart_le_actions(&req);
@@ -6707,7 +6958,13 @@ void mgmt_discoverable_timeout(struct hci_dev *hdev)
sizeof(scan), &scan);
}
update_class(&req);
- update_adv_data(&req);
+
+ /* Advertising instances don't use the global discoverable setting, so
+ * only update AD if advertising was enabled using Set Advertising.
+ */
+ if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
+ update_adv_data(&req);
+
hci_req_run(&req, NULL);

hdev->discov_timeout = 0;
@@ -7608,7 +7865,8 @@ void mgmt_reenable_advertising(struct hci_dev *hdev)
{
struct hci_request req;

- if (!hci_dev_test_flag(hdev, HCI_ADVERTISING))
+ if (!hci_dev_test_flag(hdev, HCI_ADVERTISING) &&
+ !hci_dev_test_flag(hdev, HCI_ADVERTISING_INSTANCE))
return;

hci_req_init(&req, hdev);
--
2.2.0.rc0.207.ga3a616c


2015-03-20 00:04:34

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 3/6] Bluetooth: Add data structure for advertising instance

This patch introduces a new data structure to represent advertising
instances that were added using the "Add Advertising" mgmt command.
Initially an hci_dev structure will support only one of these instances
at a time, so the current instance is simply stored as a direct member
of hci_dev.

Signed-off-by: Arman Uguray <[email protected]>
---
include/net/bluetooth/hci_core.h | 18 ++++++++++++++++++
net/bluetooth/hci_core.c | 1 +
2 files changed, 19 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b65c53d..4e32e06 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -155,6 +155,15 @@ struct oob_data {
u8 rand256[16];
};

+struct le_adv_instance {
+ __u8 index;
+ __u32 flags;
+ __u16 adv_data_len;
+ __u8 adv_data[HCI_MAX_AD_LENGTH];
+ __u16 scan_rsp_len;
+ __u8 scan_rsp_data[HCI_MAX_AD_LENGTH];
+};
+
#define HCI_MAX_SHORT_NAME_LENGTH 10

/* Default LE RPA expiry time, 15 minutes */
@@ -364,6 +373,9 @@ struct hci_dev {
__u8 scan_rsp_data[HCI_MAX_AD_LENGTH];
__u8 scan_rsp_data_len;

+ struct le_adv_instance adv_instance;
+ __u8 cur_adv_index;
+
__u8 irk[16];
__u32 rpa_timeout;
struct delayed_work rpa_expired;
@@ -550,6 +562,12 @@ static inline void hci_discovery_filter_clear(struct hci_dev *hdev)
hdev->discovery.scan_duration = 0;
}

+static inline void hci_le_adv_instance_init(struct hci_dev *hdev)
+{
+ memset(&hdev->adv_instance, 0, sizeof(struct le_adv_instance));
+ hdev->cur_adv_index = 0;
+}
+
bool hci_discovery_active(struct hci_dev *hdev);

void hci_discovery_set_state(struct hci_dev *hdev, int state);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 773f216..50a5f4c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3125,6 +3125,7 @@ struct hci_dev *hci_alloc_dev(void)

hci_init_sysfs(hdev);
discovery_init(hdev);
+ hci_le_adv_instance_init(hdev);

return hdev;
}
--
2.2.0.rc0.207.ga3a616c


2015-03-20 00:04:33

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 2/6] Bluetooth: Introduce HCI_ADVERTISING_INSTANCE setting and add AD flags

This patch introduces the HCI_ADVERTISING_INSTANCE setting, which is set
when an at least one advertising instance has been added using the
"Add Advertising" mgmt command. This patch also adds macro definitions
various advertising data fields which can be assigned using the
"Add Advertising" command.

Signed-off-by: Arman Uguray <[email protected]>
---
include/net/bluetooth/hci.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 06e7eee..e400175 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -227,6 +227,7 @@ enum {
HCI_LE_ENABLED,
HCI_ADVERTISING,
HCI_ADVERTISING_CONNECTABLE,
+ HCI_ADVERTISING_INSTANCE,
HCI_CONNECTABLE,
HCI_DISCOVERABLE,
HCI_LIMITED_DISCOVERABLE,
@@ -477,6 +478,19 @@ enum {
#define LE_AD_SIM_LE_BREDR_CTRL 0x08 /* Simultaneous LE & BR/EDR Controller */
#define LE_AD_SIM_LE_BREDR_HOST 0x10 /* Simultaneous LE & BR/EDR Host */

+/* Other Low Energy Advertising Data fielt types */
+#define LE_AD_FIELD_SOLICIT16 0x14 /* 16-bit service solic. UUIDs */
+#define LE_AD_FIELD_SOLICIT32 0x1F /* 32-bit service solic. UUIDs */
+#define LE_AD_FIELD_SOLICIT128 0x15 /* 128-bit service solic. UUIDs */
+#define LE_AD_FIELD_SVC_DATA16 0x16 /* Service Data - 16-bit UUID */
+#define LE_AD_FIELD_SVC_DATA32 0x20 /* Service Data - 32-bit UUID */
+#define LE_AD_FIELD_SVC_DATA128 0x21 /* Service Data - 128-bit UUID */
+#define LE_AD_FIELD_PUB_TRGT_ADDR 0x17 /* Public Target Address */
+#define LE_AD_FIELD_RND_TRGT_ADDR 0x18 /* Random Target Address */
+#define LE_AD_FIELD_APPEARANCE 0x19 /* Appearance */
+#define LE_AD_FIELD_MANUF_DATA 0xFF /* Manufacturer Specific Data */
+
+
/* ----- HCI Commands ---- */
#define HCI_OP_NOP 0x0000

--
2.2.0.rc0.207.ga3a616c


2015-03-20 00:04:32

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 1/6] Bluetooth: Add definitions for Add/Remove Advertising mgmt commands

This patch adds definitions for the Add Advertising and Remove
Advertising MGMT commands with stubs that initially return
"Not Supported" as command status.

Signed-off-by: Arman Uguray <[email protected]>
---
include/net/bluetooth/mgmt.h | 24 ++++++++++++++++++++++++
net/bluetooth/mgmt.c | 25 +++++++++++++++++++++++++
2 files changed, 49 insertions(+)

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index a1a6867..c60a408 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -539,6 +539,30 @@ struct mgmt_rp_read_adv_features {
__u8 instance[0];
} __packed;

+#define MGMT_OP_ADD_ADVERTISING 0x003E
+struct mgmt_cp_add_advertising {
+ __u8 instance;
+ __le32 flags;
+ __le16 duration;
+ __le16 timeout;
+ __u8 adv_data_len;
+ __u8 scan_rsp_len;
+ __u8 data[0];
+} __packed;
+#define MGMT_ADD_ADVERTISING_SIZE 11
+struct mgmt_rp_add_advertising {
+ __u8 instance;
+} __packed;
+
+#define MGMT_OP_REMOVE_ADVERTISING 0x003F
+struct mgmt_cp_remove_advertising {
+ __u8 instance;
+} __packed;
+#define MGMT_REMOVE_ADVERTISING_SIZE 1
+struct mgmt_rp_remove_advertising {
+ __u8 instance;
+} __packed;
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index f3a9579..467cac9 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -100,6 +100,8 @@ static const u16 mgmt_commands[] = {
MGMT_OP_READ_LOCAL_OOB_EXT_DATA,
MGMT_OP_READ_EXT_INDEX_LIST,
MGMT_OP_READ_ADV_FEATURES,
+ MGMT_OP_ADD_ADVERTISING,
+ MGMT_OP_REMOVE_ADVERTISING,
};

static const u16 mgmt_events[] = {
@@ -6315,6 +6317,26 @@ static int read_adv_features(struct sock *sk, struct hci_dev *hdev,
return err;
}

+static int add_advertising(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 data_len)
+{
+ BT_DBG("%s", hdev->name);
+
+ /* TODO: Implement this command. */
+ return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
+ MGMT_STATUS_NOT_SUPPORTED);
+}
+
+static int remove_advertising(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 data_len)
+{
+ BT_DBG("%s", hdev->name);
+
+ /* TODO: Implement this command. */
+ return mgmt_cmd_status(sk, hdev->id, MGMT_OP_REMOVE_ADVERTISING,
+ MGMT_STATUS_NOT_SUPPORTED);
+}
+
static const struct hci_mgmt_handler mgmt_handlers[] = {
{ NULL }, /* 0x0000 (no command) */
{ read_version, MGMT_READ_VERSION_SIZE,
@@ -6399,6 +6421,9 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
HCI_MGMT_NO_HDEV |
HCI_MGMT_UNTRUSTED },
{ read_adv_features, MGMT_READ_ADV_FEATURES_SIZE },
+ { add_advertising, MGMT_ADD_ADVERTISING_SIZE,
+ HCI_MGMT_VAR_LEN },
+ { remove_advertising, MGMT_REMOVE_ADVERTISING_SIZE },
};

void mgmt_index_added(struct hci_dev *hdev)
--
2.2.0.rc0.207.ga3a616c