2014-11-27 01:29:25

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH v11 1/3] Bluetooth: add hci_restart_le_scan

Currently there is no way to restart le scan. It's needed in
preparation for new service scan method. The way it work: it disable,
and then enable le scan on controller. During this restart special flag
is set to make sure we won't remove disable scan work from workqueue.

Signed-off-by: Jakub Pawlowski <[email protected]>
---
include/net/bluetooth/hci.h | 8 +++++++
include/net/bluetooth/hci_core.h | 4 ++++
net/bluetooth/hci_core.c | 50 +++++++++++++++++++++++++++++++++++++++-
net/bluetooth/hci_event.c | 9 +++++---
4 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index e56f909..a8784e6 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -205,6 +205,14 @@ enum {
HCI_LE_SCAN_INTERRUPTED,
};

+/*
+ * BR/EDR and/or LE discovery state flags: the flags defined here should
+ * represent state of discovery
+ */
+enum {
+ HCI_LE_SCAN_RESTARTING,
+};
+
/* A mask for the flags that are supposed to remain when a reset happens
* or the HCI device is closed.
*/
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 396c098..7e10308 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -75,6 +75,7 @@ struct discovery_state {
u32 last_adv_flags;
u8 last_adv_data[HCI_MAX_AD_LENGTH];
u8 last_adv_data_len;
+ unsigned long flags;
};

struct hci_conn_hash {
@@ -337,6 +338,7 @@ struct hci_dev {
unsigned long dev_flags;

struct delayed_work le_scan_disable;
+ struct delayed_work le_scan_restart;

__s8 adv_tx_power;
__u8 adv_data[HCI_MAX_AD_LENGTH];
@@ -1326,6 +1328,7 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event);
#define DISCOV_INTERLEAVED_TIMEOUT 5120 /* msec */
#define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04
#define DISCOV_BREDR_INQUIRY_LEN 0x08
+#define DISCOV_LE_RESTART_DELAY 300 /* msec */

int mgmt_control(struct sock *sk, struct msghdr *msg, size_t len);
int mgmt_new_settings(struct hci_dev *hdev);
@@ -1403,6 +1406,7 @@ int hci_update_random_address(struct hci_request *req, bool require_privacy,
u8 *own_addr_type);
void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
u8 *bdaddr_type);
+void hci_restart_le_scan(struct hci_dev *hdev);

#define SCO_AIRMODE_MASK 0x0003
#define SCO_AIRMODE_CVSD 0x0000
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5c319a4..97d2d35 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2557,6 +2557,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
cancel_delayed_work(&hdev->service_cache);

cancel_delayed_work_sync(&hdev->le_scan_disable);
+ cancel_delayed_work_sync(&hdev->le_scan_restart);

if (test_bit(HCI_MGMT, &hdev->dev_flags))
cancel_delayed_work_sync(&hdev->rpa_expired);
@@ -3851,13 +3852,59 @@ static void le_scan_disable_work(struct work_struct *work)

BT_DBG("%s", hdev->name);

+ cancel_delayed_work_sync(&hdev->le_scan_restart);
+
hci_req_init(&req, hdev);

hci_req_add_le_scan_disable(&req);

err = hci_req_run(&req, le_scan_disable_work_complete);
if (err)
- BT_ERR("Disable LE scanning request failed: err %d", err);
+ BT_ERR("Disable LE scan request failed: err %d", err);
+}
+
+void hci_restart_le_scan(struct hci_dev *hdev)
+{
+ queue_delayed_work(hdev->workqueue, &hdev->le_scan_restart,
+ msecs_to_jiffies(DISCOV_LE_RESTART_DELAY));
+}
+
+static void le_scan_restart_work_complete(struct hci_dev *hdev, u8 status)
+{
+ clear_bit(HCI_LE_SCAN_RESTARTING, &hdev->discovery.flags);
+
+ if (status)
+ BT_ERR("Failed to restart LE scan: status %d", status);
+}
+
+static void le_scan_restart_work(struct work_struct *work)
+{
+ struct hci_dev *hdev = container_of(work, struct hci_dev,
+ le_scan_restart.work);
+ struct hci_request req;
+ struct hci_cp_le_set_scan_enable cp;
+ int err;
+
+ BT_DBG("%s", hdev->name);
+
+ /* If controller is not scanning we are done. */
+ if (!test_bit(HCI_LE_SCAN, &hdev->dev_flags))
+ return;
+
+ hci_req_init(&req, hdev);
+
+ hci_req_add_le_scan_disable(&req);
+
+ memset(&cp, 0, sizeof(cp));
+ cp.enable = LE_SCAN_ENABLE;
+ cp.filter_dup = LE_SCAN_FILTER_DUP_ENABLE;
+ hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
+
+ set_bit(HCI_LE_SCAN_RESTARTING, &hdev->discovery.flags);
+
+ err = hci_req_run(&req, le_scan_restart_work_complete);
+ if (err)
+ BT_ERR("Restart LE scan request failed: err %d", err);
}

static void set_random_addr(struct hci_request *req, bdaddr_t *rpa)
@@ -4037,6 +4084,7 @@ struct hci_dev *hci_alloc_dev(void)
INIT_DELAYED_WORK(&hdev->power_off, hci_power_off);
INIT_DELAYED_WORK(&hdev->discov_off, hci_discov_off);
INIT_DELAYED_WORK(&hdev->le_scan_disable, le_scan_disable_work);
+ INIT_DELAYED_WORK(&hdev->le_scan_restart, le_scan_restart_work);

skb_queue_head_init(&hdev->rx_q);
skb_queue_head_init(&hdev->cmd_q);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index bd0a801..ee19656 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1157,10 +1157,13 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
d->last_adv_data_len, NULL, 0);
}

- /* Cancel this timer so that we don't try to disable scanning
- * when it's already disabled.
+ /* If HCI_LE_SCAN_RESTARTING is set, don't cancel this timer,
+ * because we're just restarting scan. Otherwise cancel it so
+ * that we don't try to disable scanning when it's already
+ * disabled.
*/
- cancel_delayed_work(&hdev->le_scan_disable);
+ if (!test_bit(HCI_LE_SCAN_RESTARTING, &hdev->discovery.flags))
+ cancel_delayed_work(&hdev->le_scan_disable);

clear_bit(HCI_LE_SCAN, &hdev->dev_flags);

--
2.2.0.rc0.207.ga3a616c



2014-11-27 12:50:18

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH v11 3/3] Bluetooth: start and stop service discovery

Hi Jakub,

On Wed, Nov 26, 2014, Jakub Pawlowski wrote:
> This patch introduces start service discovery method. The reason
> behind that is to enable users to find specific services in range
> by UUID. Whole filtering is done in mgmt_device_found.
>
> Signed-off-by: Jakub Pawlowski <[email protected]>
> ---
> include/net/bluetooth/hci.h | 1 +
> include/net/bluetooth/hci_core.h | 6 +
> include/net/bluetooth/mgmt.h | 9 ++
> net/bluetooth/hci_core.c | 1 +
> net/bluetooth/mgmt.c | 277 +++++++++++++++++++++++++++++++++++----
> 5 files changed, 267 insertions(+), 27 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index a8784e6..a88acee 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -211,6 +211,7 @@ enum {
> */
> enum {
> HCI_LE_SCAN_RESTARTING,
> + HCI_SERVICE_FILTER,
> };

At closer inspection it seems to me that this flag is redundant. Take
for example the following if-statement from later on in your patch:

> + if (!test_bit(HCI_SERVICE_FILTER, &hdev->discovery.flags) ||
> + (hdev->discovery.sd_rssi == 127 &&
> + hdev->discovery.sd_num_uuid == 0)) {
> + mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
> + return;
> + }

It should be possible to use simply the RSSI and UUID count to select
the special "service discovery" behavior. I might have overlooked
something though, in which case please enlighten me :)

> +
> + /* sd prefix stands for service discovery related properties */
> + s8 sd_rssi;
> + u16 sd_num_uuid;
> + u8 (*sd_uuid)[16];
> };

Minor cosmetic thing I know, but I'd prefer to drop the sd_ prefix from
these. I don't think we'll ever end up having similar variables but for
a different purpose in the discovery struct (and if we do then we can
always add namespacing back).

I've also corrected the parameter naming in mgmt-api.txt to match what
we use for other "lists" like Link Keys. The variables here should also
reflect that. I'd suggest as follows:

s8 rssi;
u16 uuid_count;
u8 (*uuids)[16];


> +#define MGMT_OP_START_SERVICE_DISCOVERY 0x003A
> +struct mgmt_cp_start_service_discovery {
> + __u8 type;
> + __s8 rssi_threshold;
> + __le16 num_uuid;
> + __u8 uuid[0][16];
> +} __packed;

Here you should also update the naming. I'm not sure it's worth having
the _treshold though since you're using just "rssi" elsewhere.

> +/* cleans up the state set up by the start_service_discovery function. */
> +void mgmt_clean_up_service_discovery(struct hci_dev *hdev)
> +{
> + if (!test_and_clear_bit(HCI_SERVICE_FILTER, &hdev->discovery.flags))
> + return;
> +
> + cancel_delayed_work_sync(&hdev->le_scan_restart);
> + if (hdev->discovery.sd_num_uuid > 0)
> + kfree(hdev->discovery.sd_uuid);
> + hdev->discovery.sd_num_uuid = 0;
> +}

Is this function really needed? I mean, what it does is clearly needed,
but I'd have just put it into hci_discovery_set_state() when going to
DISCOVERY_STOPPED. You could of course keep the code as a helper
function but just call it from hci_discovery_set_state(). I might have
overlooked something of course but that'd seem the most natural (and
simple) place to put this stuff.

Btw, the dependency of num_uuid to the kfree call still looks wrong to
me (I asked to change it in an earlier review). The kfree() could be
made unconditional as long as you remember to set the uuid pointer to
NULL afterwards.

> +static int init_service_discovery(struct hci_dev *hdev, s8 rssi, u16 num_uuid,
> + u8 (*uuid)[16])
> +{
> + hdev->discovery.sd_rssi = rssi;
> + hdev->discovery.sd_num_uuid = num_uuid;
> +
> + if (num_uuid > 0) {
> + hdev->discovery.sd_uuid = kmalloc(16 * num_uuid, GFP_KERNEL);
> + if (!hdev->discovery.sd_uuid)
> + return -ENOMEM;
> + memcpy(hdev->discovery.sd_uuid, uuid, 16 * num_uuid);
> + }

The memcpy and kmalloc calls could be simplified to a single kmemdup
call:

hdev->discovery.uuids = kmemdup(uuids, 16 * uuid_count, GFP_KERNEL);

> static int generic_start_discovery(struct sock *sk, struct hci_dev *hdev,
> void *data, u16 len, u16 opcode)
> {
> - struct mgmt_cp_start_discovery *cp = data;
> struct pending_cmd *cmd;
> struct hci_cp_le_set_scan_param param_cp;
> struct hci_cp_le_set_scan_enable enable_cp;
> struct hci_cp_inquiry inq_cp;
> struct hci_request req;
> /* General inquiry access code (GIAC) */
> + s8 sd_rssi = 0;
> u8 lap[3] = { 0x33, 0x8b, 0x9e };
> - u8 status, own_addr_type;
> + u8 status, own_addr_type, type;
> + u8 (*sd_uuid)[16] = NULL;
> + u16 sd_num_uuid = 0;
> int err;
>
> BT_DBG("%s", hdev->name);
>
> + if (opcode == MGMT_OP_START_SERVICE_DISCOVERY) {
> + struct mgmt_cp_start_service_discovery *cp = data;
> + u16 expected_len, num_uuid_tmp;
> +
> + type = cp->type;
> + num_uuid_tmp = __le16_to_cpu(cp->num_uuid);
> + expected_len = sizeof(*cp) + num_uuid_tmp * 16;
> +
> + if (expected_len != len) {
> + return cmd_complete(sk, hdev->id, opcode,
> + MGMT_STATUS_INVALID_PARAMS, &type,
> + sizeof(type));
> + }

No need for the { } in a one-statement branch. Also, the num_uuid_tmp
variable seems unnecessary here. Can't you just directly assign to your
sd_num_uuid variable? (which btw should be renamed to uuid_count)

> + sd_rssi = cp->rssi_threshold;
> + sd_num_uuid = num_uuid_tmp;
> + if (sd_num_uuid > 0)
> + sd_uuid = cp->uuid;
> + } else {
> + struct mgmt_cp_start_discovery *cp = data;
> +
> + type = cp->type;
> + }

Since you've got a dedicated branch for each type of discovery we could
avoid the sd_num_uuid initialization to 0 upon declaration (something
that's in general discouraged whenever possible). The you'd just set it
to the __le16_to_cpu() result in the first branch and 0 in the second.

Johan

2014-11-27 12:22:22

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH v11 1/3] Bluetooth: add hci_restart_le_scan

Hi Jakub,

On Wed, Nov 26, 2014, Jakub Pawlowski wrote:
> +/*
> + * BR/EDR and/or LE discovery state flags: the flags defined here should
> + * represent state of discovery
> + */
> +enum {
> + HCI_LE_SCAN_RESTARTING,
> +};

I think this enum should be as close as possible to the struct
discovery_state definition, i.e. right before it in hci_core.h. The
hci.h file should be strictly for HCI definitions only, but for
historical reasons it also contains some implementation specific stuff.

Also note that the net/ coding style doesn't use an empty first line for
multi-line comments.

> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -75,6 +75,7 @@ struct discovery_state {
> u32 last_adv_flags;
> u8 last_adv_data[HCI_MAX_AD_LENGTH];
> u8 last_adv_data_len;
> + unsigned long flags;
> };
>
> struct hci_conn_hash {
> @@ -337,6 +338,7 @@ struct hci_dev {
> unsigned long dev_flags;
>
> struct delayed_work le_scan_disable;
> + struct delayed_work le_scan_restart;
>
> __s8 adv_tx_power;
> __u8 adv_data[HCI_MAX_AD_LENGTH];
> @@ -1326,6 +1328,7 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event);
> #define DISCOV_INTERLEAVED_TIMEOUT 5120 /* msec */
> #define DISCOV_INTERLEAVED_INQUIRY_LEN 0x04
> #define DISCOV_BREDR_INQUIRY_LEN 0x08
> +#define DISCOV_LE_RESTART_DELAY 300 /* msec */
>
> int mgmt_control(struct sock *sk, struct msghdr *msg, size_t len);
> int mgmt_new_settings(struct hci_dev *hdev);
> @@ -1403,6 +1406,7 @@ int hci_update_random_address(struct hci_request *req, bool require_privacy,
> u8 *own_addr_type);
> void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> u8 *bdaddr_type);
> +void hci_restart_le_scan(struct hci_dev *hdev);

When looking at your later patches (mainly 3/3) this
hci_restart_le_scan() functionality is only accessed from within mgmt.c.
I'd therefore suggest to simply have it all as part of mgmt.c (as static
functions) and only later export it through hci_core.h if there really
is a need.

Johan

2014-11-27 01:29:27

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH v11 3/3] Bluetooth: start and stop service discovery

This patch introduces start service discovery method. The reason
behind that is to enable users to find specific services in range
by UUID. Whole filtering is done in mgmt_device_found.

Signed-off-by: Jakub Pawlowski <[email protected]>
---
include/net/bluetooth/hci.h | 1 +
include/net/bluetooth/hci_core.h | 6 +
include/net/bluetooth/mgmt.h | 9 ++
net/bluetooth/hci_core.c | 1 +
net/bluetooth/mgmt.c | 277 +++++++++++++++++++++++++++++++++++----
5 files changed, 267 insertions(+), 27 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index a8784e6..a88acee 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -211,6 +211,7 @@ enum {
*/
enum {
HCI_LE_SCAN_RESTARTING,
+ HCI_SERVICE_FILTER,
};

/* A mask for the flags that are supposed to remain when a reset happens
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 7e10308..8017808 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -76,6 +76,11 @@ struct discovery_state {
u8 last_adv_data[HCI_MAX_AD_LENGTH];
u8 last_adv_data_len;
unsigned long flags;
+
+ /* sd prefix stands for service discovery related properties */
+ s8 sd_rssi;
+ u16 sd_num_uuid;
+ u8 (*sd_uuid)[16];
};

struct hci_conn_hash {
@@ -1396,6 +1401,7 @@ void mgmt_new_conn_param(struct hci_dev *hdev, bdaddr_t *bdaddr,
u16 max_interval, u16 latency, u16 timeout);
void mgmt_reenable_advertising(struct hci_dev *hdev);
void mgmt_smp_complete(struct hci_conn *conn, bool complete);
+void mgmt_clean_up_service_discovery(struct hci_dev *hdev);

u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index b391fd6..dc21a5a 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -495,6 +495,15 @@ struct mgmt_cp_set_public_address {
} __packed;
#define MGMT_SET_PUBLIC_ADDRESS_SIZE 6

+#define MGMT_OP_START_SERVICE_DISCOVERY 0x003A
+struct mgmt_cp_start_service_discovery {
+ __u8 type;
+ __s8 rssi_threshold;
+ __le16 num_uuid;
+ __u8 uuid[0][16];
+} __packed;
+#define MGMT_START_SERVICE_DISCOVERY_SIZE 4
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 97d2d35..c56c132 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3853,6 +3853,7 @@ static void le_scan_disable_work(struct work_struct *work)
BT_DBG("%s", hdev->name);

cancel_delayed_work_sync(&hdev->le_scan_restart);
+ mgmt_clean_up_service_discovery(hdev);

hci_req_init(&req, hdev);

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3e2ba4c..f6d9da7 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -93,6 +93,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_READ_CONFIG_INFO,
MGMT_OP_SET_EXTERNAL_CONFIG,
MGMT_OP_SET_PUBLIC_ADDRESS,
+ MGMT_OP_START_SERVICE_DISCOVERY,
};

static const u16 mgmt_events[] = {
@@ -1258,6 +1259,18 @@ static void clean_up_hci_complete(struct hci_dev *hdev, u8 status)
}
}

+/* cleans up the state set up by the start_service_discovery function. */
+void mgmt_clean_up_service_discovery(struct hci_dev *hdev)
+{
+ if (!test_and_clear_bit(HCI_SERVICE_FILTER, &hdev->discovery.flags))
+ return;
+
+ cancel_delayed_work_sync(&hdev->le_scan_restart);
+ if (hdev->discovery.sd_num_uuid > 0)
+ kfree(hdev->discovery.sd_uuid);
+ hdev->discovery.sd_num_uuid = 0;
+}
+
static bool hci_stop_discovery(struct hci_request *req)
{
struct hci_dev *hdev = req->hdev;
@@ -1270,6 +1283,7 @@ static bool hci_stop_discovery(struct hci_request *req)
hci_req_add(req, HCI_OP_INQUIRY_CANCEL, 0, NULL);
} else {
cancel_delayed_work(&hdev->le_scan_disable);
+ mgmt_clean_up_service_discovery(hdev);
hci_req_add_le_scan_disable(req);
}

@@ -3737,42 +3751,83 @@ static void start_discovery_complete(struct hci_dev *hdev, u8 status)
queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable, timeout);
}

+static int init_service_discovery(struct hci_dev *hdev, s8 rssi, u16 num_uuid,
+ u8 (*uuid)[16])
+{
+ hdev->discovery.sd_rssi = rssi;
+ hdev->discovery.sd_num_uuid = num_uuid;
+
+ if (num_uuid > 0) {
+ hdev->discovery.sd_uuid = kmalloc(16 * num_uuid, GFP_KERNEL);
+ if (!hdev->discovery.sd_uuid)
+ return -ENOMEM;
+ memcpy(hdev->discovery.sd_uuid, uuid, 16 * num_uuid);
+ }
+
+ set_bit(HCI_SERVICE_FILTER, &hdev->discovery.flags);
+ return 0;
+}
+
static int generic_start_discovery(struct sock *sk, struct hci_dev *hdev,
void *data, u16 len, u16 opcode)
{
- struct mgmt_cp_start_discovery *cp = data;
struct pending_cmd *cmd;
struct hci_cp_le_set_scan_param param_cp;
struct hci_cp_le_set_scan_enable enable_cp;
struct hci_cp_inquiry inq_cp;
struct hci_request req;
/* General inquiry access code (GIAC) */
+ s8 sd_rssi = 0;
u8 lap[3] = { 0x33, 0x8b, 0x9e };
- u8 status, own_addr_type;
+ u8 status, own_addr_type, type;
+ u8 (*sd_uuid)[16] = NULL;
+ u16 sd_num_uuid = 0;
int err;

BT_DBG("%s", hdev->name);

+ if (opcode == MGMT_OP_START_SERVICE_DISCOVERY) {
+ struct mgmt_cp_start_service_discovery *cp = data;
+ u16 expected_len, num_uuid_tmp;
+
+ type = cp->type;
+ num_uuid_tmp = __le16_to_cpu(cp->num_uuid);
+ expected_len = sizeof(*cp) + num_uuid_tmp * 16;
+
+ if (expected_len != len) {
+ return cmd_complete(sk, hdev->id, opcode,
+ MGMT_STATUS_INVALID_PARAMS, &type,
+ sizeof(type));
+ }
+
+ sd_rssi = cp->rssi_threshold;
+ sd_num_uuid = num_uuid_tmp;
+ if (sd_num_uuid > 0)
+ sd_uuid = cp->uuid;
+ } else {
+ struct mgmt_cp_start_discovery *cp = data;
+
+ type = cp->type;
+ }
+
hci_dev_lock(hdev);

if (!hdev_is_powered(hdev)) {
err = cmd_complete(sk, hdev->id, opcode,
- MGMT_STATUS_NOT_POWERED,
- &cp->type, sizeof(cp->type));
+ MGMT_STATUS_NOT_POWERED, &type,
+ sizeof(type));
goto failed;
}

if (test_bit(HCI_PERIODIC_INQ, &hdev->dev_flags)) {
err = cmd_complete(sk, hdev->id, opcode,
- MGMT_STATUS_BUSY, &cp->type,
- sizeof(cp->type));
+ MGMT_STATUS_BUSY, &type, sizeof(type));
goto failed;
}

if (hdev->discovery.state != DISCOVERY_STOPPED) {
err = cmd_complete(sk, hdev->id, opcode,
- MGMT_STATUS_BUSY, &cp->type,
- sizeof(cp->type));
+ MGMT_STATUS_BUSY, &type, sizeof(type));
goto failed;
}

@@ -3782,7 +3837,7 @@ static int generic_start_discovery(struct sock *sk, struct hci_dev *hdev,
goto failed;
}

- hdev->discovery.type = cp->type;
+ hdev->discovery.type = type;

hci_req_init(&req, hdev);

@@ -3790,16 +3845,16 @@ static int generic_start_discovery(struct sock *sk, struct hci_dev *hdev,
case DISCOV_TYPE_BREDR:
status = mgmt_bredr_support(hdev);
if (status) {
- err = cmd_complete(sk, hdev->id, opcode, status,
- &cp->type, sizeof(cp->type));
+ err = cmd_complete(sk, hdev->id, opcode, status, &type,
+ sizeof(type));
mgmt_pending_remove(cmd);
goto failed;
}

if (test_bit(HCI_INQUIRY, &hdev->flags)) {
err = cmd_complete(sk, hdev->id, opcode,
- MGMT_STATUS_BUSY, &cp->type,
- sizeof(cp->type));
+ MGMT_STATUS_BUSY, &type,
+ sizeof(type));
mgmt_pending_remove(cmd);
goto failed;
}
@@ -3816,8 +3871,8 @@ static int generic_start_discovery(struct sock *sk, struct hci_dev *hdev,
case DISCOV_TYPE_INTERLEAVED:
status = mgmt_le_support(hdev);
if (status) {
- err = cmd_complete(sk, hdev->id, opcode, status,
- &cp->type, sizeof(cp->type));
+ err = cmd_complete(sk, hdev->id, opcode, status, &type,
+ sizeof(type));
mgmt_pending_remove(cmd);
goto failed;
}
@@ -3825,8 +3880,8 @@ static int generic_start_discovery(struct sock *sk, struct hci_dev *hdev,
if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
err = cmd_complete(sk, hdev->id, opcode,
- MGMT_STATUS_NOT_SUPPORTED,
- &cp->type, sizeof(cp->type));
+ MGMT_STATUS_NOT_SUPPORTED, &type,
+ sizeof(type));
mgmt_pending_remove(cmd);
goto failed;
}
@@ -3839,9 +3894,8 @@ static int generic_start_discovery(struct sock *sk, struct hci_dev *hdev,
if (hci_conn_hash_lookup_state(hdev, LE_LINK,
BT_CONNECT)) {
err = cmd_complete(sk, hdev->id, opcode,
- MGMT_STATUS_REJECTED,
- &cp->type,
- sizeof(cp->type));
+ MGMT_STATUS_REJECTED, &type,
+ sizeof(type));
mgmt_pending_remove(cmd);
goto failed;
}
@@ -3865,8 +3919,8 @@ static int generic_start_discovery(struct sock *sk, struct hci_dev *hdev,
err = hci_update_random_address(&req, true, &own_addr_type);
if (err < 0) {
err = cmd_complete(sk, hdev->id, opcode,
- MGMT_STATUS_FAILED,
- &cp->type, sizeof(cp->type));
+ MGMT_STATUS_FAILED, &type,
+ sizeof(type));
mgmt_pending_remove(cmd);
goto failed;
}
@@ -3887,13 +3941,21 @@ static int generic_start_discovery(struct sock *sk, struct hci_dev *hdev,

default:
err = cmd_complete(sk, hdev->id, opcode,
- MGMT_STATUS_INVALID_PARAMS,
- &cp->type, sizeof(cp->type));
+ MGMT_STATUS_INVALID_PARAMS, &type,
+ sizeof(type));
mgmt_pending_remove(cmd);
goto failed;
}

+ if (opcode == MGMT_OP_START_SERVICE_DISCOVERY) {
+ err = init_service_discovery(hdev, sd_rssi, sd_num_uuid,
+ sd_uuid);
+ if (err)
+ goto failed;
+ }
+
err = hci_req_run(&req, start_discovery_complete);
+
if (err < 0)
mgmt_pending_remove(cmd);
else
@@ -5685,6 +5747,13 @@ unlock:
return err;
}

+static int start_service_discovery(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 len)
+{
+ return generic_start_discovery(sk, hdev, data, len,
+ MGMT_OP_START_SERVICE_DISCOVERY);
+}
+
static const struct mgmt_handler {
int (*func) (struct sock *sk, struct hci_dev *hdev, void *data,
u16 data_len);
@@ -5749,6 +5818,7 @@ static const struct mgmt_handler {
{ read_config_info, false, MGMT_READ_CONFIG_INFO_SIZE },
{ set_external_config, false, MGMT_SET_EXTERNAL_CONFIG_SIZE },
{ set_public_address, false, MGMT_SET_PUBLIC_ADDRESS_SIZE },
+ { start_service_discovery, true, MGMT_START_SERVICE_DISCOVERY_SIZE },
};

int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
@@ -6815,6 +6885,127 @@ void mgmt_read_local_oob_data_complete(struct hci_dev *hdev, u8 *hash192,
mgmt_pending_remove(cmd);
}

+struct parsed_uuid {
+ struct list_head list;
+ u8 uuid[16];
+};
+
+/* this is reversed hex representation of bluetooth base uuid. We need it for
+ * service uuid parsing in eir.
+ */
+static const u8 reverse_base_uuid[] = {
+ 0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00, 0x80,
+ 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+
+static int add_uuid_to_list(struct list_head *uuids, u8 *uuid)
+{
+ struct parsed_uuid *tmp_uuid;
+
+ tmp_uuid = kmalloc(sizeof(*tmp_uuid), GFP_KERNEL);
+ if (tmp_uuid == NULL)
+ return -ENOMEM;
+
+ memcpy(tmp_uuid->uuid, uuid, 16);
+ INIT_LIST_HEAD(&tmp_uuid->list);
+ list_add(&tmp_uuid->list, uuids);
+ return 0;
+}
+
+static void free_uuids_list(struct list_head *uuids)
+{
+ struct parsed_uuid *uuid, *tmp;
+
+ list_for_each_entry_safe(uuid, tmp, uuids, list) {
+ __list_del_entry(&uuid->list);
+ kfree(uuid);
+ }
+}
+
+static int eir_parse(u8 *eir, u8 eir_len, struct list_head *uuids)
+{
+ size_t offset;
+ u8 uuid[16];
+ int i, ret;
+
+ offset = 0;
+ while (offset < eir_len) {
+ uint8_t field_len = eir[0];
+
+ /* Check for the end of EIR */
+ if (field_len == 0)
+ break;
+
+ if (offset + field_len > eir_len)
+ return -EINVAL;
+
+ switch (eir[1]) {
+ case EIR_UUID16_ALL:
+ case EIR_UUID16_SOME:
+ for (i = 0; i + 3 <= field_len; i += 2) {
+ memcpy(uuid, reverse_base_uuid, 16);
+ uuid[13] = eir[i + 3];
+ uuid[12] = eir[i + 2];
+ ret = add_uuid_to_list(uuids, uuid);
+ if (ret)
+ return ret;
+ }
+ break;
+ case EIR_UUID32_ALL:
+ case EIR_UUID32_SOME:
+ for (i = 0; i + 5 <= field_len; i += 4) {
+ memcpy(uuid, reverse_base_uuid, 16);
+ uuid[15] = eir[i + 5];
+ uuid[14] = eir[i + 4];
+ uuid[13] = eir[i + 3];
+ uuid[12] = eir[i + 2];
+ ret = add_uuid_to_list(uuids, uuid);
+ if (ret)
+ return ret;
+ }
+ break;
+ case EIR_UUID128_ALL:
+ case EIR_UUID128_SOME:
+ for (i = 0; i + 17 <= field_len; i += 16) {
+ memcpy(uuid, eir + i + 2, 16);
+ ret = add_uuid_to_list(uuids, uuid);
+ if (ret)
+ return ret;
+ }
+ break;
+ }
+
+ offset += field_len + 1;
+ eir += field_len + 1;
+ }
+ return 0;
+}
+
+enum {
+ NO_MATCH,
+ SERVICE_MATCH,
+ FULL_MATCH
+};
+
+static u8 find_matches(struct hci_dev *hdev, s8 rssi, struct list_head *uuids)
+{
+ struct parsed_uuid *uuidptr, *tmp_uuid;
+ int i, match_type = NO_MATCH, min_rssi = hdev->discovery.sd_rssi;
+
+ list_for_each_entry_safe(uuidptr, tmp_uuid, uuids, list) {
+ for (i = 0; i < hdev->discovery.sd_num_uuid; i++) {
+ u8 *filter_uuid = hdev->discovery.sd_uuid[i];
+
+ if (memcmp(filter_uuid, uuidptr->uuid, 16) != 0)
+ continue;
+ if (rssi >= min_rssi)
+ return FULL_MATCH;
+ match_type = SERVICE_MATCH;
+ }
+ }
+ return match_type;
+}
+
void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
u8 addr_type, u8 *dev_class, s8 rssi, u32 flags,
u8 *eir, u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len)
@@ -6822,6 +7013,9 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
char buf[512];
struct mgmt_ev_device_found *ev = (void *) buf;
size_t ev_size;
+ LIST_HEAD(uuids);
+ int err = 0;
+ u8 match_type;

/* Don't send events for a non-kernel initiated discovery. With
* LE one exception is if we have pend_le_reports > 0 in which
@@ -6860,7 +7054,31 @@ void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
ev->eir_len = cpu_to_le16(eir_len + scan_rsp_len);
ev_size = sizeof(*ev) + eir_len + scan_rsp_len;

- mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
+ if (!test_bit(HCI_SERVICE_FILTER, &hdev->discovery.flags) ||
+ (hdev->discovery.sd_rssi == 127 &&
+ hdev->discovery.sd_num_uuid == 0)) {
+ mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev, ev_size, NULL);
+ return;
+ }
+
+ err = eir_parse(eir, eir_len, &uuids);
+ if (err) {
+ free_uuids_list(&uuids);
+ return;
+ }
+
+ match_type = find_matches(hdev, rssi, &uuids);
+ free_uuids_list(&uuids);
+
+ if (match_type == NO_MATCH)
+ return;
+
+ if (test_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks))
+ hci_restart_le_scan(hdev);
+
+ if (match_type == FULL_MATCH)
+ mgmt_event(MGMT_EV_DEVICE_FOUND, hdev, ev,
+ ev_size, NULL);
}

void mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
@@ -6893,10 +7111,15 @@ void mgmt_discovering(struct hci_dev *hdev, u8 discovering)

BT_DBG("%s discovering %u", hdev->name, discovering);

- if (discovering)
+ if (discovering) {
cmd = mgmt_pending_find(MGMT_OP_START_DISCOVERY, hdev);
- else
+ if (cmd == NULL)
+ cmd = mgmt_pending_find(MGMT_OP_START_SERVICE_DISCOVERY,
+ hdev);
+
+ } else {
cmd = mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, hdev);
+ }

if (cmd != NULL) {
u8 type = hdev->discovery.type;
--
2.2.0.rc0.207.ga3a616c


2014-11-27 01:29:26

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH v11 2/3] Bluetooth: Extract generic start and stop discovery

This commit extract generic_start_discovery and generic_stop_discovery
in preparation for start and stop service discovery. The reason behind
that is that both functions will share big part of code, and it would
be much easier to maintain just one generic method.

Signed-off-by: Jakub Pawlowski <[email protected]>
---
net/bluetooth/mgmt.c | 47 +++++++++++++++++++++++++----------------------
1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index cbeef5f..3e2ba4c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3675,7 +3675,8 @@ done:
return err;
}

-static int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status)
+static int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status,
+ u16 opcode)
{
struct pending_cmd *cmd;
u8 type;
@@ -3683,7 +3684,7 @@ static int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status)

hci_discovery_set_state(hdev, DISCOVERY_STOPPED);

- cmd = mgmt_pending_find(MGMT_OP_START_DISCOVERY, hdev);
+ cmd = mgmt_pending_find(opcode, hdev);
if (!cmd)
return -ENOENT;

@@ -3704,7 +3705,8 @@ static void start_discovery_complete(struct hci_dev *hdev, u8 status)

if (status) {
hci_dev_lock(hdev);
- mgmt_start_discovery_failed(hdev, status);
+ mgmt_start_discovery_failed(hdev, status,
+ MGMT_OP_START_DISCOVERY);
hci_dev_unlock(hdev);
return;
}
@@ -3735,8 +3737,8 @@ static void start_discovery_complete(struct hci_dev *hdev, u8 status)
queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable, timeout);
}

-static int start_discovery(struct sock *sk, struct hci_dev *hdev,
- void *data, u16 len)
+static int generic_start_discovery(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 len, u16 opcode)
{
struct mgmt_cp_start_discovery *cp = data;
struct pending_cmd *cmd;
@@ -3754,27 +3756,27 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
hci_dev_lock(hdev);

if (!hdev_is_powered(hdev)) {
- err = cmd_complete(sk, hdev->id, MGMT_OP_START_DISCOVERY,
+ err = cmd_complete(sk, hdev->id, opcode,
MGMT_STATUS_NOT_POWERED,
&cp->type, sizeof(cp->type));
goto failed;
}

if (test_bit(HCI_PERIODIC_INQ, &hdev->dev_flags)) {
- err = cmd_complete(sk, hdev->id, MGMT_OP_START_DISCOVERY,
+ err = cmd_complete(sk, hdev->id, opcode,
MGMT_STATUS_BUSY, &cp->type,
sizeof(cp->type));
goto failed;
}

if (hdev->discovery.state != DISCOVERY_STOPPED) {
- err = cmd_complete(sk, hdev->id, MGMT_OP_START_DISCOVERY,
+ err = cmd_complete(sk, hdev->id, opcode,
MGMT_STATUS_BUSY, &cp->type,
sizeof(cp->type));
goto failed;
}

- cmd = mgmt_pending_add(sk, MGMT_OP_START_DISCOVERY, hdev, NULL, 0);
+ cmd = mgmt_pending_add(sk, opcode, hdev, NULL, 0);
if (!cmd) {
err = -ENOMEM;
goto failed;
@@ -3788,16 +3790,14 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
case DISCOV_TYPE_BREDR:
status = mgmt_bredr_support(hdev);
if (status) {
- err = cmd_complete(sk, hdev->id,
- MGMT_OP_START_DISCOVERY, status,
+ err = cmd_complete(sk, hdev->id, opcode, status,
&cp->type, sizeof(cp->type));
mgmt_pending_remove(cmd);
goto failed;
}

if (test_bit(HCI_INQUIRY, &hdev->flags)) {
- err = cmd_complete(sk, hdev->id,
- MGMT_OP_START_DISCOVERY,
+ err = cmd_complete(sk, hdev->id, opcode,
MGMT_STATUS_BUSY, &cp->type,
sizeof(cp->type));
mgmt_pending_remove(cmd);
@@ -3816,8 +3816,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
case DISCOV_TYPE_INTERLEAVED:
status = mgmt_le_support(hdev);
if (status) {
- err = cmd_complete(sk, hdev->id,
- MGMT_OP_START_DISCOVERY, status,
+ err = cmd_complete(sk, hdev->id, opcode, status,
&cp->type, sizeof(cp->type));
mgmt_pending_remove(cmd);
goto failed;
@@ -3825,8 +3824,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,

if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
- err = cmd_complete(sk, hdev->id,
- MGMT_OP_START_DISCOVERY,
+ err = cmd_complete(sk, hdev->id, opcode,
MGMT_STATUS_NOT_SUPPORTED,
&cp->type, sizeof(cp->type));
mgmt_pending_remove(cmd);
@@ -3840,8 +3838,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
*/
if (hci_conn_hash_lookup_state(hdev, LE_LINK,
BT_CONNECT)) {
- err = cmd_complete(sk, hdev->id,
- MGMT_OP_START_DISCOVERY,
+ err = cmd_complete(sk, hdev->id, opcode,
MGMT_STATUS_REJECTED,
&cp->type,
sizeof(cp->type));
@@ -3867,8 +3864,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
*/
err = hci_update_random_address(&req, true, &own_addr_type);
if (err < 0) {
- err = cmd_complete(sk, hdev->id,
- MGMT_OP_START_DISCOVERY,
+ err = cmd_complete(sk, hdev->id, opcode,
MGMT_STATUS_FAILED,
&cp->type, sizeof(cp->type));
mgmt_pending_remove(cmd);
@@ -3890,7 +3886,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
break;

default:
- err = cmd_complete(sk, hdev->id, MGMT_OP_START_DISCOVERY,
+ err = cmd_complete(sk, hdev->id, opcode,
MGMT_STATUS_INVALID_PARAMS,
&cp->type, sizeof(cp->type));
mgmt_pending_remove(cmd);
@@ -3908,6 +3904,13 @@ failed:
return err;
}

+static int start_discovery(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 len)
+{
+ return generic_start_discovery(sk, hdev, data, len,
+ MGMT_OP_START_DISCOVERY);
+}
+
static int mgmt_stop_discovery_failed(struct hci_dev *hdev, u8 status)
{
struct pending_cmd *cmd;
--
2.2.0.rc0.207.ga3a616c