2014-05-09 19:35:27

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 0/6] Get Connection Information

Hi,

Here are v2 patches which implement Get Connection Information mgmt command.
As in v1, patches 1-5 provide implementation based on API v2 and patch 6
updates to API v3.


Changes in v2:
- fixed comments regarding naming
- removed hci_request patch and used hci_sent_cmd_data() to retrieve hci_conn
it works fine and code is quite simple, but it looks a bit hackish imo
- some functions are renamed and comments added, hopefully request flow is
now easier to follow


Andrzej Kaczmarek (6):
Bluetooth: Store TX power level for connection
Bluetooth: Move eir_append_data up
Bluetooth: Add support to get connection information
Bluetooth: Make max age for connection information configurable
Bluetooth: Avoid pooling TX power for LE links
Bluetooth: Add support for max_tx_power in Get Conn Info

include/net/bluetooth/hci.h | 11 ++
include/net/bluetooth/hci_core.h | 8 ++
include/net/bluetooth/mgmt.h | 17 +++
net/bluetooth/hci_conn.c | 2 +
net/bluetooth/hci_core.c | 4 +
net/bluetooth/hci_event.c | 45 +++++++
net/bluetooth/mgmt.c | 250 +++++++++++++++++++++++++++++++++++++--
7 files changed, 326 insertions(+), 11 deletions(-)

--
1.9.2


2014-05-10 17:49:30

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 3/6] Bluetooth: Add support to get connection information

Hi Andrzej,

>>> This patch adds support for Get Connection Information mgmt command
>>> which can be used to query for information about connection, i.e. RSSI
>>> or local TX power level.
>>>
>>> RSSI is returned as response parameter while any other information is
>>> returned in standard EIR format.
>>>
>>> Signed-off-by: Andrzej Kaczmarek <[email protected]>
>>> ---
>>> include/net/bluetooth/hci_core.h | 2 +
>>> include/net/bluetooth/mgmt.h | 16 +++
>>> net/bluetooth/mgmt.c | 216 +++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 234 insertions(+)
>>>
>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>> index 211bad6..9be523a 100644
>>> --- a/include/net/bluetooth/hci_core.h
>>> +++ b/include/net/bluetooth/hci_core.h
>>> @@ -378,6 +378,8 @@ struct hci_conn {
>>> __s8 tx_power;
>>> unsigned long flags;
>>>
>>> + unsigned long conn_info_timestamp;
>>> +
>>> __u8 remote_cap;
>>> __u8 remote_auth;
>>> __u8 remote_id;
>>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
>>> index d4b571c..34faa2e 100644
>>> --- a/include/net/bluetooth/mgmt.h
>>> +++ b/include/net/bluetooth/mgmt.h
>>> @@ -409,6 +409,22 @@ struct mgmt_cp_load_irks {
>>> } __packed;
>>> #define MGMT_LOAD_IRKS_SIZE 2
>>>
>>> +#define MGMT_CONN_INFO_DATA_TX_POWER 0x00000001
>>> +
>>> +#define MGMT_OP_GET_CONN_INFO 0x0031
>>> +struct mgmt_cp_get_conn_info {
>>> + struct mgmt_addr_info addr;
>>> + __u32 data_type;
>>
>> the current idea is to remove the data_type and just provide the address information in the command.
>>
>>> +} __packed;
>>> +#define MGMT_GET_CONN_INFO_SIZE (MGMT_ADDR_INFO_SIZE + 4)
>>> +struct mgmt_rp_get_conn_info {
>>> + struct mgmt_addr_info addr;
>>> + __s8 rssi;
>>
>> lets just add __s8 tx_power and __s8 max_tx_power here and that is it. No extra flags or any other details.
>>> + __u32 flags;
>>> + __le16 eir_len;
>>> + __u8 eir[0];
>>> +} __packed;
>>
>> The EIR looked like a good idea in the beginning, but it seems not so much of a good idea anymore.
>>
>>> +
>>> #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 5e88ac9..2d86ce1 100644
>>> --- a/net/bluetooth/mgmt.c
>>> +++ b/net/bluetooth/mgmt.c
>>> @@ -83,6 +83,7 @@ static const u16 mgmt_commands[] = {
>>> MGMT_OP_SET_DEBUG_KEYS,
>>> MGMT_OP_SET_PRIVACY,
>>> MGMT_OP_LOAD_IRKS,
>>> + MGMT_OP_GET_CONN_INFO,
>>> };
>>>
>>> static const u16 mgmt_events[] = {
>>> @@ -113,6 +114,8 @@ static const u16 mgmt_events[] = {
>>>
>>> #define CACHE_TIMEOUT msecs_to_jiffies(2 * 1000)
>>>
>>> +#define CONN_INFO_MAX_AGE msecs_to_jiffies(2 * 1000)
>>> +
>>> #define hdev_is_powered(hdev) (test_bit(HCI_UP, &hdev->flags) && \
>>> !test_bit(HCI_AUTO_OFF, &hdev->dev_flags))
>>>
>>> @@ -4568,6 +4571,218 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
>>> return err;
>>> }
>>>
>>> +struct cmd_conn_lookup {
>>> + struct hci_conn *conn;
>>> + u8 mgmt_status;
>>> +};
>>> +
>>> +static void get_conn_info_complete(struct pending_cmd *cmd, void *data)
>>> +{
>>> + struct cmd_conn_lookup *match = data;
>>> + struct mgmt_cp_get_conn_info *cp;
>>> + struct mgmt_rp_get_conn_info *rp;
>>> + char buf[sizeof(*rp) + 3];
>>> + struct hci_conn *conn = cmd->user_data;
>>> + __u16 eir_len = 0;
>>> +
>>> + if (conn != match->conn)
>>> + return;
>>> +
>>> + cp = (struct mgmt_cp_get_conn_info *) cmd->param;
>>> + rp = (struct mgmt_rp_get_conn_info *) buf;
>>> +
>>> + memset(buf, 0, sizeof(buf));
>>> + bacpy(&rp->addr.bdaddr, &cp->addr.bdaddr);
>>> + rp->addr.type = cp->addr.type;
>>
>> actually I would prefer if we use HCI_TX_POWER_INVALID here and also set the RSSI to a default value. I think it is 0 for BR/EDR and some magic value for LE.
>>
>>> +
>>> + if (match->mgmt_status) {
>>> + cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO,
>>> + match->mgmt_status, rp, sizeof(*rp));
>>> + goto done;
>>> + }
>>> +
>>> + rp->rssi = conn->rssi;
>>> +
>>> + if (cp->data_type & MGMT_CONN_INFO_DATA_TX_POWER) {
>>> + eir_len = eir_append_data(rp->eir, 0, EIR_TX_POWER,
>>> + &conn->tx_power,
>>> + sizeof(conn->tx_power));
>>> +
>>> + rp->eir_len = cpu_to_le16(eir_len);
>>> + }
>>> +
>>> + cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO,
>>> + MGMT_STATUS_SUCCESS, rp, sizeof(*rp) + eir_len);
>>> +
>>> +done:
>>> + hci_conn_drop(conn);
>>> +
>>> + mgmt_pending_remove(cmd);
>>> +}
>>> +
>>> +static void conn_info_refresh_complete(struct hci_dev *hdev, u8 status)
>>> +{
>>> + struct hci_cp_read_rssi *cp;
>>> + struct hci_conn *conn;
>>> + struct cmd_conn_lookup match;
>>> + u16 handle;
>>> +
>>> + BT_DBG("status 0x%02x", status);
>>> +
>>> + hci_dev_lock(hdev);
>>> +
>>> + /* Last command sent in request should be Read RSSI, but if it isn't
>>> + * then we look for Read Transmit Power Level which is other command
>>> + * sent in request and probably failed.
>>> + *
>>> + * Both commands have handle as first parameter so it's safe to cast
>>> + * data on the same command struct.
>>> + */
>>
>> I do not like this order. I like it RSSI, TX Power and then TX Max Power. Only if RSSI fails we should return an error. The others should still succeed.
>>
>>> + cp = hci_sent_cmd_data(hdev, HCI_OP_READ_RSSI);
>>> + if (!cp)
>>> + cp = hci_sent_cmd_data(hdev, HCI_OP_READ_TX_POWER_LEVEL);
>>> +
>>> + if (!cp) {
>>> + BT_ERR("invalid sent_cmd in response");
>>> + goto unlock;
>>> + }
>>> +
>>> + handle = __le16_to_cpu(cp->handle);
>>> + conn = hci_conn_hash_lookup_handle(hdev, handle);
>>> + if (!conn) {
>>> + BT_ERR("unknown handle (%d) in response", handle);
>>> + goto unlock;
>>> + }
>>> +
>>> + if (!status)
>>> + conn->conn_info_timestamp = jiffies;
>>> +
>>> + match.conn = conn;
>>> + match.mgmt_status = mgmt_status(status);
>>> +
>>> + /* Cache refresh is complete, now reply for each pending request for
>>> + * given connection.
>>> + */
>>> + mgmt_pending_foreach(MGMT_OP_GET_CONN_INFO, hdev,
>>> + get_conn_info_complete, &match);
>>> +
>>> +unlock:
>>> + hci_dev_unlock(hdev);
>>> +}
>>> +
>>> +static bool check_pending_conn_info(struct hci_dev *hdev, struct hci_conn *conn)
>>> +{
>>> + struct pending_cmd *cmd;
>>> +
>>> + list_for_each_entry(cmd, &hdev->mgmt_pending, list) {
>>> + if (cmd->opcode == MGMT_OP_GET_CONN_INFO &&
>>> + cmd->user_data == conn)
>>> + return true;
>>> + }
>>> +
>>> + return false;
>>> +}
>>> +
>>> +static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,
>>> + u16 len)
>>> +{
>>> + struct mgmt_cp_get_conn_info *cp = data;
>>> + struct mgmt_rp_get_conn_info rp;
>>> + struct hci_conn *conn;
>>> + struct pending_cmd *cmd;
>>> + struct hci_request req;
>>> + bool is_pending;
>>> + int err = 0;
>>> +
>>> + BT_DBG("%s", hdev->name);
>>> +
>>> + memset(&rp, 0, sizeof(rp));
>>> + bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
>>> + rp.addr.type = cp->addr.type;
>>
>> We need to fill in proper defaults that indicate that the values are invalid. See above comment.
>>
>>> +
>>> + if (!bdaddr_type_is_valid(cp->addr.type))
>>> + return cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
>>> + MGMT_STATUS_INVALID_PARAMS,
>>> + &rp, sizeof(rp));
>>> +
>>> + hci_dev_lock(hdev);
>>> +
>>> + if (!hdev_is_powered(hdev)) {
>>> + err = cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
>>> + MGMT_STATUS_NOT_POWERED, &rp, sizeof(rp));
>>> + goto unlock;
>>> + }
>>> +
>>> + if (cp->addr.type == BDADDR_BREDR)
>>> + conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,
>>> + &cp->addr.bdaddr);
>>> + else
>>> + conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);
>>> +
>>> + if (!conn || conn->state != BT_CONNECTED) {
>>> + err = cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
>>> + MGMT_STATUS_NOT_CONNECTED, &rp, sizeof(rp));
>>> + goto unlock;
>>> + }
>>> +
>>> + /* We need to check if there's another pending request for this conn
>>> + * so we don't query controller again - just add another pending and
>>> + * return.
>>> + */
>>> + is_pending = check_pending_conn_info(hdev, conn);
>>
>> I wonder if we should be really naive here and not try too outsmart ourselves. Maybe we should just do this timestamp based. If you happen to call the same mgmt command while we are executing one to get new values, you are just out of luck and get the old caches values.
>
> Removing this won't really simplify code, it's just few lines of code
> which will be removed. And you can imagine (unlikely to happen, but
> still...) scenario in which two clients are pooling for conn info on
> different devices at the same time and somehow 2nd is always out of
> luck - it won't never get up-to-date values because only 1st device is
> refreshed. I'd keep this.

it is not about the lines of code, it is about the logic behind to make this work. We already have a bit of complex handling in mgmt. If by any means possible, I want to make it simple and not more complex.

My thinking currently is like this:

mgmt_get_conn_info:

hci_lock

if (timestamp_is_expired)
set_timestamp_to_current
get_new_conn_info
else
return_cached_values

hci_unlock

There might be a really out-of-luck situation where you always get the second last accurate information, but who cares. Next time it asks they will be updated since we are updating the process.

To make it less unlikely, we can just add a bit of randomness into the age of the information. Which I would have proposed anyway so that current callers can not guess when to poll again. Via debugfs we might just expose a min_age and max_age value to configure.

Regards

Marcel


2014-05-09 22:39:02

by Andrzej Kaczmarek

[permalink] [raw]
Subject: Re: [PATCH v2 3/6] Bluetooth: Add support to get connection information

Hi Marcel,

On 9 May 2014 23:26, Marcel Holtmann <[email protected]> wrote:
> Hi Andrzej,
>
>> This patch adds support for Get Connection Information mgmt command
>> which can be used to query for information about connection, i.e. RSSI
>> or local TX power level.
>>
>> RSSI is returned as response parameter while any other information is
>> returned in standard EIR format.
>>
>> Signed-off-by: Andrzej Kaczmarek <[email protected]>
>> ---
>> include/net/bluetooth/hci_core.h | 2 +
>> include/net/bluetooth/mgmt.h | 16 +++
>> net/bluetooth/mgmt.c | 216 +++++++++++++++++++++++++++++++++=
++++++
>> 3 files changed, 234 insertions(+)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hc=
i_core.h
>> index 211bad6..9be523a 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -378,6 +378,8 @@ struct hci_conn {
>> __s8 tx_power;
>> unsigned long flags;
>>
>> + unsigned long conn_info_timestamp;
>> +
>> __u8 remote_cap;
>> __u8 remote_auth;
>> __u8 remote_id;
>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
>> index d4b571c..34faa2e 100644
>> --- a/include/net/bluetooth/mgmt.h
>> +++ b/include/net/bluetooth/mgmt.h
>> @@ -409,6 +409,22 @@ struct mgmt_cp_load_irks {
>> } __packed;
>> #define MGMT_LOAD_IRKS_SIZE 2
>>
>> +#define MGMT_CONN_INFO_DATA_TX_POWER 0x00000001
>> +
>> +#define MGMT_OP_GET_CONN_INFO 0x0031
>> +struct mgmt_cp_get_conn_info {
>> + struct mgmt_addr_info addr;
>> + __u32 data_type;
>
> the current idea is to remove the data_type and just provide the address =
information in the command.
>
>> +} __packed;
>> +#define MGMT_GET_CONN_INFO_SIZE (MGMT_ADDR_INFO_SIZE + 4)
>> +struct mgmt_rp_get_conn_info {
>> + struct mgmt_addr_info addr;
>> + __s8 rssi;
>
> lets just add __s8 tx_power and __s8 max_tx_power here and that is it. No=
extra flags or any other details.
>> + __u32 flags;
>> + __le16 eir_len;
>> + __u8 eir[0];
>> +} __packed;
>
> The EIR looked like a good idea in the beginning, but it seems not so muc=
h of a good idea anymore.
>
>> +
>> #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 5e88ac9..2d86ce1 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -83,6 +83,7 @@ static const u16 mgmt_commands[] =3D {
>> MGMT_OP_SET_DEBUG_KEYS,
>> MGMT_OP_SET_PRIVACY,
>> MGMT_OP_LOAD_IRKS,
>> + MGMT_OP_GET_CONN_INFO,
>> };
>>
>> static const u16 mgmt_events[] =3D {
>> @@ -113,6 +114,8 @@ static const u16 mgmt_events[] =3D {
>>
>> #define CACHE_TIMEOUT msecs_to_jiffies(2 * 1000)
>>
>> +#define CONN_INFO_MAX_AGE msecs_to_jiffies(2 * 1000)
>> +
>> #define hdev_is_powered(hdev) (test_bit(HCI_UP, &hdev->flags) && \
>> !test_bit(HCI_AUTO_OFF, &hdev->dev_flags))
>>
>> @@ -4568,6 +4571,218 @@ static int load_long_term_keys(struct sock *sk, =
struct hci_dev *hdev,
>> return err;
>> }
>>
>> +struct cmd_conn_lookup {
>> + struct hci_conn *conn;
>> + u8 mgmt_status;
>> +};
>> +
>> +static void get_conn_info_complete(struct pending_cmd *cmd, void *data)
>> +{
>> + struct cmd_conn_lookup *match =3D data;
>> + struct mgmt_cp_get_conn_info *cp;
>> + struct mgmt_rp_get_conn_info *rp;
>> + char buf[sizeof(*rp) + 3];
>> + struct hci_conn *conn =3D cmd->user_data;
>> + __u16 eir_len =3D 0;
>> +
>> + if (conn !=3D match->conn)
>> + return;
>> +
>> + cp =3D (struct mgmt_cp_get_conn_info *) cmd->param;
>> + rp =3D (struct mgmt_rp_get_conn_info *) buf;
>> +
>> + memset(buf, 0, sizeof(buf));
>> + bacpy(&rp->addr.bdaddr, &cp->addr.bdaddr);
>> + rp->addr.type =3D cp->addr.type;
>
> actually I would prefer if we use HCI_TX_POWER_INVALID here and also set =
the RSSI to a default value. I think it is 0 for BR/EDR and some magic valu=
e for LE.
>
>> +
>> + if (match->mgmt_status) {
>> + cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO,
>> + match->mgmt_status, rp, sizeof(*rp));
>> + goto done;
>> + }
>> +
>> + rp->rssi =3D conn->rssi;
>> +
>> + if (cp->data_type & MGMT_CONN_INFO_DATA_TX_POWER) {
>> + eir_len =3D eir_append_data(rp->eir, 0, EIR_TX_POWER,
>> + &conn->tx_power,
>> + sizeof(conn->tx_power));
>> +
>> + rp->eir_len =3D cpu_to_le16(eir_len);
>> + }
>> +
>> + cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO,
>> + MGMT_STATUS_SUCCESS, rp, sizeof(*rp) + eir_len);
>> +
>> +done:
>> + hci_conn_drop(conn);
>> +
>> + mgmt_pending_remove(cmd);
>> +}
>> +
>> +static void conn_info_refresh_complete(struct hci_dev *hdev, u8 status)
>> +{
>> + struct hci_cp_read_rssi *cp;
>> + struct hci_conn *conn;
>> + struct cmd_conn_lookup match;
>> + u16 handle;
>> +
>> + BT_DBG("status 0x%02x", status);
>> +
>> + hci_dev_lock(hdev);
>> +
>> + /* Last command sent in request should be Read RSSI, but if it isn=
't
>> + * then we look for Read Transmit Power Level which is other comma=
nd
>> + * sent in request and probably failed.
>> + *
>> + * Both commands have handle as first parameter so it's safe to ca=
st
>> + * data on the same command struct.
>> + */
>
> I do not like this order. I like it RSSI, TX Power and then TX Max Power.=
Only if RSSI fails we should return an error. The others should still succ=
eed.
>
>> + cp =3D hci_sent_cmd_data(hdev, HCI_OP_READ_RSSI);
>> + if (!cp)
>> + cp =3D hci_sent_cmd_data(hdev, HCI_OP_READ_TX_POWER_LEVEL)=
;
>> +
>> + if (!cp) {
>> + BT_ERR("invalid sent_cmd in response");
>> + goto unlock;
>> + }
>> +
>> + handle =3D __le16_to_cpu(cp->handle);
>> + conn =3D hci_conn_hash_lookup_handle(hdev, handle);
>> + if (!conn) {
>> + BT_ERR("unknown handle (%d) in response", handle);
>> + goto unlock;
>> + }
>> +
>> + if (!status)
>> + conn->conn_info_timestamp =3D jiffies;
>> +
>> + match.conn =3D conn;
>> + match.mgmt_status =3D mgmt_status(status);
>> +
>> + /* Cache refresh is complete, now reply for each pending request f=
or
>> + * given connection.
>> + */
>> + mgmt_pending_foreach(MGMT_OP_GET_CONN_INFO, hdev,
>> + get_conn_info_complete, &match);
>> +
>> +unlock:
>> + hci_dev_unlock(hdev);
>> +}
>> +
>> +static bool check_pending_conn_info(struct hci_dev *hdev, struct hci_co=
nn *conn)
>> +{
>> + struct pending_cmd *cmd;
>> +
>> + list_for_each_entry(cmd, &hdev->mgmt_pending, list) {
>> + if (cmd->opcode =3D=3D MGMT_OP_GET_CONN_INFO &&
>> + cmd->user_data =3D=3D conn)
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *d=
ata,
>> + u16 len)
>> +{
>> + struct mgmt_cp_get_conn_info *cp =3D data;
>> + struct mgmt_rp_get_conn_info rp;
>> + struct hci_conn *conn;
>> + struct pending_cmd *cmd;
>> + struct hci_request req;
>> + bool is_pending;
>> + int err =3D 0;
>> +
>> + BT_DBG("%s", hdev->name);
>> +
>> + memset(&rp, 0, sizeof(rp));
>> + bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
>> + rp.addr.type =3D cp->addr.type;
>
> We need to fill in proper defaults that indicate that the values are inva=
lid. See above comment.
>
>> +
>> + if (!bdaddr_type_is_valid(cp->addr.type))
>> + return cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
>> + MGMT_STATUS_INVALID_PARAMS,
>> + &rp, sizeof(rp));
>> +
>> + hci_dev_lock(hdev);
>> +
>> + if (!hdev_is_powered(hdev)) {
>> + err =3D cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
>> + MGMT_STATUS_NOT_POWERED, &rp, sizeof(rp=
));
>> + goto unlock;
>> + }
>> +
>> + if (cp->addr.type =3D=3D BDADDR_BREDR)
>> + conn =3D hci_conn_hash_lookup_ba(hdev, ACL_LINK,
>> + &cp->addr.bdaddr);
>> + else
>> + conn =3D hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.=
bdaddr);
>> +
>> + if (!conn || conn->state !=3D BT_CONNECTED) {
>> + err =3D cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
>> + MGMT_STATUS_NOT_CONNECTED, &rp, sizeof(=
rp));
>> + goto unlock;
>> + }
>> +
>> + /* We need to check if there's another pending request for this co=
nn
>> + * so we don't query controller again - just add another pending a=
nd
>> + * return.
>> + */
>> + is_pending =3D check_pending_conn_info(hdev, conn);
>
> I wonder if we should be really naive here and not try too outsmart ourse=
lves. Maybe we should just do this timestamp based. If you happen to call t=
he same mgmt command while we are executing one to get new values, you are =
just out of luck and get the old caches values.

Removing this won't really simplify code, it's just few lines of code
which will be removed. And you can imagine (unlikely to happen, but
still...) scenario in which two clients are pooling for conn info on
different devices at the same time and somehow 2nd is always out of
luck - it won't never get up-to-date values because only 1st device is
refreshed. I'd keep this.

BR,
Andrzej

2014-05-09 21:28:00

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] Bluetooth: Make max age for connection information configurable

Hi Andrzej,

> This patch adds debugfs entry to configure max age of connection
> information. Cached values will be only refreshed by Get Connection
> Information request if cache is older than this value. Default value
> is 2000ms.
>
> Signed-off-by: Andrzej Kaczmarek <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 4 ++++
> net/bluetooth/hci_core.c | 4 ++++
> net/bluetooth/mgmt.c | 4 +---
> 3 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 9be523a..6353617 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -145,6 +145,9 @@ struct oob_data {
> /* Default LE RPA expiry time, 15 minutes */
> #define HCI_DEFAULT_RPA_TIMEOUT (15 * 60)
>
> +/* Default max age of connection information, 2 sec */
> +#define DEFAULT_CONN_INFO_MAX_AGE 2000
> +
> struct amp_assoc {
> __u16 len;
> __u16 offset;
> @@ -200,6 +203,7 @@ struct hci_dev {
> __u16 le_conn_min_interval;
> __u16 le_conn_max_interval;
> __u16 discov_interleaved_timeout;
> + __u16 conn_info_max_age;
> __u8 ssp_debug_mode;
>
> __u16 devid_source;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index d31f144..b2d1eee1e3 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1754,6 +1754,9 @@ static int __hci_init(struct hci_dev *hdev)
> &blacklist_fops);
> debugfs_create_file("uuids", 0444, hdev->debugfs, hdev, &uuids_fops);
>
> + debugfs_create_u16("conn_info_max_age", 0644, hdev->debugfs,
> + &hdev->conn_info_max_age);
> +
> if (lmp_bredr_capable(hdev)) {
> debugfs_create_file("inquiry_cache", 0444, hdev->debugfs,
> hdev, &inquiry_cache_fops);
> @@ -3789,6 +3792,7 @@ struct hci_dev *hci_alloc_dev(void)
>
> hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
> hdev->discov_interleaved_timeout = DISCOV_INTERLEAVED_TIMEOUT;
> + hdev->conn_info_max_age = DEFAULT_CONN_INFO_MAX_AGE;
>
> mutex_init(&hdev->lock);
> mutex_init(&hdev->req_lock);
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 2d86ce1..a7f45d2 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -114,8 +114,6 @@ static const u16 mgmt_events[] = {
>
> #define CACHE_TIMEOUT msecs_to_jiffies(2 * 1000)
>
> -#define CONN_INFO_MAX_AGE msecs_to_jiffies(2 * 1000)
> -

so instead of introducing it here and the later moving it around. Lets just get this right first and introduce the debugfs setting first before using the value.

Regards

Marcel


2014-05-09 21:26:36

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 3/6] Bluetooth: Add support to get connection information

Hi Andrzej,

> This patch adds support for Get Connection Information mgmt command
> which can be used to query for information about connection, i.e. RSSI
> or local TX power level.
>
> RSSI is returned as response parameter while any other information is
> returned in standard EIR format.
>
> Signed-off-by: Andrzej Kaczmarek <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 2 +
> include/net/bluetooth/mgmt.h | 16 +++
> net/bluetooth/mgmt.c | 216 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 234 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 211bad6..9be523a 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -378,6 +378,8 @@ struct hci_conn {
> __s8 tx_power;
> unsigned long flags;
>
> + unsigned long conn_info_timestamp;
> +
> __u8 remote_cap;
> __u8 remote_auth;
> __u8 remote_id;
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index d4b571c..34faa2e 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -409,6 +409,22 @@ struct mgmt_cp_load_irks {
> } __packed;
> #define MGMT_LOAD_IRKS_SIZE 2
>
> +#define MGMT_CONN_INFO_DATA_TX_POWER 0x00000001
> +
> +#define MGMT_OP_GET_CONN_INFO 0x0031
> +struct mgmt_cp_get_conn_info {
> + struct mgmt_addr_info addr;
> + __u32 data_type;

the current idea is to remove the data_type and just provide the address information in the command.

> +} __packed;
> +#define MGMT_GET_CONN_INFO_SIZE (MGMT_ADDR_INFO_SIZE + 4)
> +struct mgmt_rp_get_conn_info {
> + struct mgmt_addr_info addr;
> + __s8 rssi;

lets just add __s8 tx_power and __s8 max_tx_power here and that is it. No extra flags or any other details.
> + __u32 flags;
> + __le16 eir_len;
> + __u8 eir[0];
> +} __packed;

The EIR looked like a good idea in the beginning, but it seems not so much of a good idea anymore.

> +
> #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 5e88ac9..2d86ce1 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -83,6 +83,7 @@ static const u16 mgmt_commands[] = {
> MGMT_OP_SET_DEBUG_KEYS,
> MGMT_OP_SET_PRIVACY,
> MGMT_OP_LOAD_IRKS,
> + MGMT_OP_GET_CONN_INFO,
> };
>
> static const u16 mgmt_events[] = {
> @@ -113,6 +114,8 @@ static const u16 mgmt_events[] = {
>
> #define CACHE_TIMEOUT msecs_to_jiffies(2 * 1000)
>
> +#define CONN_INFO_MAX_AGE msecs_to_jiffies(2 * 1000)
> +
> #define hdev_is_powered(hdev) (test_bit(HCI_UP, &hdev->flags) && \
> !test_bit(HCI_AUTO_OFF, &hdev->dev_flags))
>
> @@ -4568,6 +4571,218 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
> return err;
> }
>
> +struct cmd_conn_lookup {
> + struct hci_conn *conn;
> + u8 mgmt_status;
> +};
> +
> +static void get_conn_info_complete(struct pending_cmd *cmd, void *data)
> +{
> + struct cmd_conn_lookup *match = data;
> + struct mgmt_cp_get_conn_info *cp;
> + struct mgmt_rp_get_conn_info *rp;
> + char buf[sizeof(*rp) + 3];
> + struct hci_conn *conn = cmd->user_data;
> + __u16 eir_len = 0;
> +
> + if (conn != match->conn)
> + return;
> +
> + cp = (struct mgmt_cp_get_conn_info *) cmd->param;
> + rp = (struct mgmt_rp_get_conn_info *) buf;
> +
> + memset(buf, 0, sizeof(buf));
> + bacpy(&rp->addr.bdaddr, &cp->addr.bdaddr);
> + rp->addr.type = cp->addr.type;

actually I would prefer if we use HCI_TX_POWER_INVALID here and also set the RSSI to a default value. I think it is 0 for BR/EDR and some magic value for LE.

> +
> + if (match->mgmt_status) {
> + cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO,
> + match->mgmt_status, rp, sizeof(*rp));
> + goto done;
> + }
> +
> + rp->rssi = conn->rssi;
> +
> + if (cp->data_type & MGMT_CONN_INFO_DATA_TX_POWER) {
> + eir_len = eir_append_data(rp->eir, 0, EIR_TX_POWER,
> + &conn->tx_power,
> + sizeof(conn->tx_power));
> +
> + rp->eir_len = cpu_to_le16(eir_len);
> + }
> +
> + cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO,
> + MGMT_STATUS_SUCCESS, rp, sizeof(*rp) + eir_len);
> +
> +done:
> + hci_conn_drop(conn);
> +
> + mgmt_pending_remove(cmd);
> +}
> +
> +static void conn_info_refresh_complete(struct hci_dev *hdev, u8 status)
> +{
> + struct hci_cp_read_rssi *cp;
> + struct hci_conn *conn;
> + struct cmd_conn_lookup match;
> + u16 handle;
> +
> + BT_DBG("status 0x%02x", status);
> +
> + hci_dev_lock(hdev);
> +
> + /* Last command sent in request should be Read RSSI, but if it isn't
> + * then we look for Read Transmit Power Level which is other command
> + * sent in request and probably failed.
> + *
> + * Both commands have handle as first parameter so it's safe to cast
> + * data on the same command struct.
> + */

I do not like this order. I like it RSSI, TX Power and then TX Max Power. Only if RSSI fails we should return an error. The others should still succeed.

> + cp = hci_sent_cmd_data(hdev, HCI_OP_READ_RSSI);
> + if (!cp)
> + cp = hci_sent_cmd_data(hdev, HCI_OP_READ_TX_POWER_LEVEL);
> +
> + if (!cp) {
> + BT_ERR("invalid sent_cmd in response");
> + goto unlock;
> + }
> +
> + handle = __le16_to_cpu(cp->handle);
> + conn = hci_conn_hash_lookup_handle(hdev, handle);
> + if (!conn) {
> + BT_ERR("unknown handle (%d) in response", handle);
> + goto unlock;
> + }
> +
> + if (!status)
> + conn->conn_info_timestamp = jiffies;
> +
> + match.conn = conn;
> + match.mgmt_status = mgmt_status(status);
> +
> + /* Cache refresh is complete, now reply for each pending request for
> + * given connection.
> + */
> + mgmt_pending_foreach(MGMT_OP_GET_CONN_INFO, hdev,
> + get_conn_info_complete, &match);
> +
> +unlock:
> + hci_dev_unlock(hdev);
> +}
> +
> +static bool check_pending_conn_info(struct hci_dev *hdev, struct hci_conn *conn)
> +{
> + struct pending_cmd *cmd;
> +
> + list_for_each_entry(cmd, &hdev->mgmt_pending, list) {
> + if (cmd->opcode == MGMT_OP_GET_CONN_INFO &&
> + cmd->user_data == conn)
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,
> + u16 len)
> +{
> + struct mgmt_cp_get_conn_info *cp = data;
> + struct mgmt_rp_get_conn_info rp;
> + struct hci_conn *conn;
> + struct pending_cmd *cmd;
> + struct hci_request req;
> + bool is_pending;
> + int err = 0;
> +
> + BT_DBG("%s", hdev->name);
> +
> + memset(&rp, 0, sizeof(rp));
> + bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
> + rp.addr.type = cp->addr.type;

We need to fill in proper defaults that indicate that the values are invalid. See above comment.

> +
> + if (!bdaddr_type_is_valid(cp->addr.type))
> + return cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
> + MGMT_STATUS_INVALID_PARAMS,
> + &rp, sizeof(rp));
> +
> + hci_dev_lock(hdev);
> +
> + if (!hdev_is_powered(hdev)) {
> + err = cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
> + MGMT_STATUS_NOT_POWERED, &rp, sizeof(rp));
> + goto unlock;
> + }
> +
> + if (cp->addr.type == BDADDR_BREDR)
> + conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,
> + &cp->addr.bdaddr);
> + else
> + conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);
> +
> + if (!conn || conn->state != BT_CONNECTED) {
> + err = cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
> + MGMT_STATUS_NOT_CONNECTED, &rp, sizeof(rp));
> + goto unlock;
> + }
> +
> + /* We need to check if there's another pending request for this conn
> + * so we don't query controller again - just add another pending and
> + * return.
> + */
> + is_pending = check_pending_conn_info(hdev, conn);

I wonder if we should be really naive here and not try too outsmart ourselves. Maybe we should just do this timestamp based. If you happen to call the same mgmt command while we are executing one to get new values, you are just out of luck and get the old caches values.

> +
> + cmd = mgmt_pending_add(sk, MGMT_OP_GET_CONN_INFO, hdev,
> + data, len);
> + if (!cmd) {
> + err = -ENOMEM;
> + goto unlock;
> + }
> +
> + hci_conn_hold(conn);
> + cmd->user_data = conn;
> +
> + /* There was already pending request for this connection, no need to
> + * start another request to controller.
> + */
> + if (is_pending)
> + goto unlock;
> +
> + /* Query controller to refresh cached values if they are too old or
> + * unknown, otherwise just reply.
> + */
> + if (time_after(jiffies, conn->conn_info_timestamp +
> + CONN_INFO_MAX_AGE) ||
> + conn->tx_power == HCI_TX_POWER_INVALID) {
> + struct hci_cp_read_tx_power_level req_txp_cp;
> + struct hci_cp_read_rssi req_rssi_cp;
> +
> + hci_req_init(&req, hdev);
> +
> + req_txp_cp.handle = cpu_to_le16(conn->handle);
> + req_txp_cp.type = 0x00;
> + hci_req_add(&req, HCI_OP_READ_TX_POWER_LEVEL,
> + sizeof(req_txp_cp), &req_txp_cp);
> +
> + req_rssi_cp.handle = cpu_to_le16(conn->handle);
> + hci_req_add(&req, HCI_OP_READ_RSSI, sizeof(req_rssi_cp),
> + &req_rssi_cp);
> +
> + err = hci_req_run(&req, conn_info_refresh_complete);
> + if (err < 0) {
> + hci_conn_drop(conn);
> + mgmt_pending_remove(cmd);
> + }
> + } else {
> + struct cmd_conn_lookup match = { conn, MGMT_STATUS_SUCCESS };
> +
> + get_conn_info_complete(cmd, &match);
> + }
> +
> +unlock:
> + hci_dev_unlock(hdev);
> + return err;
> +}
> +
> static const struct mgmt_handler {
> int (*func) (struct sock *sk, struct hci_dev *hdev, void *data,
> u16 data_len);
> @@ -4623,6 +4838,7 @@ static const struct mgmt_handler {
> { set_debug_keys, false, MGMT_SETTING_SIZE },
> { set_privacy, false, MGMT_SET_PRIVACY_SIZE },
> { load_irks, true, MGMT_LOAD_IRKS_SIZE },
> + { get_conn_info, false, MGMT_GET_CONN_INFO_SIZE },
> };

Regards

Marcel


2014-05-09 21:17:22

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] Bluetooth: Store TX power level for connection

Hi Andrzej,

> This patch adds support to store local TX power level for connection
> when reply for HCI_Read_Transmit_Power_Level is received.
>
> Signed-off-by: Andrzej Kaczmarek <[email protected]>
> ---
> include/net/bluetooth/hci.h | 11 +++++++++++
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_conn.c | 1 +
> net/bluetooth/hci_event.c | 29 +++++++++++++++++++++++++++++
> 4 files changed, 42 insertions(+)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index ad2ecc9..201a09a 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1054,6 +1054,17 @@ struct hci_cp_write_page_scan_activity {
> __le16 window;
> } __packed;
>
> +#define HCI_OP_READ_TX_POWER_LEVEL 0x0c2d
> +struct hci_cp_read_tx_power_level {
> + __le16 handle;
> + __u8 type;
> +} __packed;
> +struct hci_rp_read_tx_power_level {
> + __u8 status;
> + __le16 handle;
> + __s8 tx_power_level;
> +} __packed;

I applied this patch to bluetooth-next tree, but I replaced tx_power_level with tx_power to make it more consistent with what all the other tx_power structs and defines have.

Regards

Marcel


2014-05-09 21:10:33

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] Bluetooth: Add support for max_tx_power in Get Conn Info

Hi Andrzej,

> This patch adds support for max_tx_power in Get Connection Information
> request. Value is read only once for given connection and then always
> returned in response as parameter.
>
> Signed-off-by: Andrzej Kaczmarek <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> include/net/bluetooth/mgmt.h | 1 +
> net/bluetooth/hci_conn.c | 1 +
> net/bluetooth/hci_event.c | 18 +++++++++++++++++-
> net/bluetooth/mgmt.c | 8 ++++++++
> 5 files changed, 28 insertions(+), 1 deletion(-)

I would prefer if you split the actual tracking of the value from the mgmt part of it. Keep it two patches. The first one can be applied easily before the second until we sort out the details of the mgmt API.

> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 6353617..665752c 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -380,6 +380,7 @@ struct hci_conn {
> __u16 le_conn_max_interval;
> __s8 rssi;
> __s8 tx_power;
> + __s8 max_tx_power;
> unsigned long flags;
>
> unsigned long conn_info_timestamp;
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 34faa2e..33d016b 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -420,6 +420,7 @@ struct mgmt_cp_get_conn_info {
> struct mgmt_rp_get_conn_info {
> struct mgmt_addr_info addr;
> __s8 rssi;
> + __s8 max_tx_power;
> __u32 flags;
> __le16 eir_len;
> __u8 eir[0];
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 74b368b..a987e7d 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -408,6 +408,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
> conn->remote_auth = 0xff;
> conn->key_type = 0xff;
> conn->tx_power = HCI_TX_POWER_INVALID;
> + conn->max_tx_power = HCI_TX_POWER_INVALID;
>
> set_bit(HCI_CONN_POWER_SAVE, &conn->flags);
> conn->disc_timeout = HCI_DISCONN_TIMEOUT;
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 63cc9bc..af4276d 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1283,9 +1283,25 @@ static void hci_cc_read_tx_power_level(struct hci_dev *hdev,
> hci_dev_lock(hdev);
>
> conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(rp->handle));
> - if (conn && sent->type == 0x00)
> + if (!conn)
> + goto unlock;
> +
> + switch (sent->type) {
> + case 0x00:
> conn->tx_power = rp->tx_power_level;
> + break;
> +
> + case 0x01:
> + conn->max_tx_power = rp->tx_power_level;
> + break;
> +

Remove these extra empty lines between the case statement. They are just bloat.

> + default:
> + BT_ERR("Used reserved Read_Transmit_Power_Level param %d",
> + sent->type);

I do not think we want to print an error here. We are just tracking type we know. So remove the default case.

> + break;
> + }
>
> +unlock:
> hci_dev_unlock(hdev);
> }
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index cac2edf..fa918f7 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -4600,6 +4600,7 @@ static void get_conn_info_complete(struct pending_cmd *cmd, void *data)
> }
>
> rp->rssi = conn->rssi;
> + rp->max_tx_power = conn->max_tx_power;
>
> if (cp->data_type & MGMT_CONN_INFO_DATA_TX_POWER) {
> eir_len = eir_append_data(rp->eir, 0, EIR_TX_POWER,
> @@ -4767,6 +4768,13 @@ static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,
> sizeof(req_txp_cp), &req_txp_cp);
> }
>
> + if (conn->max_tx_power == HCI_TX_POWER_INVALID) {
> + req_txp_cp.handle = cpu_to_le16(conn->handle);
> + req_txp_cp.type = 0x01;
> + hci_req_add(&req, HCI_OP_READ_TX_POWER_LEVEL,
> + sizeof(req_txp_cp), &req_txp_cp);
> + }
> +

If the TX power can not change for LE, then neither can the TX max power.

> req_rssi_cp.handle = cpu_to_le16(conn->handle);
> hci_req_add(&req, HCI_OP_READ_RSSI, sizeof(req_rssi_cp),
> &req_rssi_cp);

I prefer if we read the RSSI before reading anything else. Actually the order should be RSSI, TX power and then TX max power.

For RSSI we always have to return and if it fails, we can just error out. That is what the hci_req_run will do if any of the HCI commands fail. If they fail, we can easily just return HCI_TX_POWER_INVALID for the other two values.

Regards

Marcel


2014-05-09 19:35:33

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 6/6] Bluetooth: Add support for max_tx_power in Get Conn Info

This patch adds support for max_tx_power in Get Connection Information
request. Value is read only once for given connection and then always
returned in response as parameter.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
include/net/bluetooth/mgmt.h | 1 +
net/bluetooth/hci_conn.c | 1 +
net/bluetooth/hci_event.c | 18 +++++++++++++++++-
net/bluetooth/mgmt.c | 8 ++++++++
5 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 6353617..665752c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -380,6 +380,7 @@ struct hci_conn {
__u16 le_conn_max_interval;
__s8 rssi;
__s8 tx_power;
+ __s8 max_tx_power;
unsigned long flags;

unsigned long conn_info_timestamp;
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 34faa2e..33d016b 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -420,6 +420,7 @@ struct mgmt_cp_get_conn_info {
struct mgmt_rp_get_conn_info {
struct mgmt_addr_info addr;
__s8 rssi;
+ __s8 max_tx_power;
__u32 flags;
__le16 eir_len;
__u8 eir[0];
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 74b368b..a987e7d 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -408,6 +408,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
conn->remote_auth = 0xff;
conn->key_type = 0xff;
conn->tx_power = HCI_TX_POWER_INVALID;
+ conn->max_tx_power = HCI_TX_POWER_INVALID;

set_bit(HCI_CONN_POWER_SAVE, &conn->flags);
conn->disc_timeout = HCI_DISCONN_TIMEOUT;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 63cc9bc..af4276d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1283,9 +1283,25 @@ static void hci_cc_read_tx_power_level(struct hci_dev *hdev,
hci_dev_lock(hdev);

conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(rp->handle));
- if (conn && sent->type == 0x00)
+ if (!conn)
+ goto unlock;
+
+ switch (sent->type) {
+ case 0x00:
conn->tx_power = rp->tx_power_level;
+ break;
+
+ case 0x01:
+ conn->max_tx_power = rp->tx_power_level;
+ break;
+
+ default:
+ BT_ERR("Used reserved Read_Transmit_Power_Level param %d",
+ sent->type);
+ break;
+ }

+unlock:
hci_dev_unlock(hdev);
}

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index cac2edf..fa918f7 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4600,6 +4600,7 @@ static void get_conn_info_complete(struct pending_cmd *cmd, void *data)
}

rp->rssi = conn->rssi;
+ rp->max_tx_power = conn->max_tx_power;

if (cp->data_type & MGMT_CONN_INFO_DATA_TX_POWER) {
eir_len = eir_append_data(rp->eir, 0, EIR_TX_POWER,
@@ -4767,6 +4768,13 @@ static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,
sizeof(req_txp_cp), &req_txp_cp);
}

+ if (conn->max_tx_power == HCI_TX_POWER_INVALID) {
+ req_txp_cp.handle = cpu_to_le16(conn->handle);
+ req_txp_cp.type = 0x01;
+ hci_req_add(&req, HCI_OP_READ_TX_POWER_LEVEL,
+ sizeof(req_txp_cp), &req_txp_cp);
+ }
+
req_rssi_cp.handle = cpu_to_le16(conn->handle);
hci_req_add(&req, HCI_OP_READ_RSSI, sizeof(req_rssi_cp),
&req_rssi_cp);
--
1.9.2

2014-05-09 19:35:32

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 5/6] Bluetooth: Avoid pooling TX power for LE links

TX power for LE links is immutable thus we do not need to query for it
if already have value.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
---
net/bluetooth/mgmt.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a7f45d2..cac2edf 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4756,10 +4756,16 @@ static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,

hci_req_init(&req, hdev);

- req_txp_cp.handle = cpu_to_le16(conn->handle);
- req_txp_cp.type = 0x00;
- hci_req_add(&req, HCI_OP_READ_TX_POWER_LEVEL,
- sizeof(req_txp_cp), &req_txp_cp);
+ /* For LE links TX power is immutable so we don't need to query
+ * controller again once value is known.
+ */
+ if (!bdaddr_type_is_le(cp->addr.type) ||
+ conn->tx_power == HCI_TX_POWER_INVALID) {
+ req_txp_cp.handle = cpu_to_le16(conn->handle);
+ req_txp_cp.type = 0x00;
+ hci_req_add(&req, HCI_OP_READ_TX_POWER_LEVEL,
+ sizeof(req_txp_cp), &req_txp_cp);
+ }

req_rssi_cp.handle = cpu_to_le16(conn->handle);
hci_req_add(&req, HCI_OP_READ_RSSI, sizeof(req_rssi_cp),
--
1.9.2

2014-05-09 19:35:29

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 2/6] Bluetooth: Move eir_append_data up

This patch moves eir_append_data up in file, close to other eir-related
functions, so we can use it later in mgmt handlers.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index f2a9422..5e88ac9 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -701,6 +701,17 @@ static void update_adv_data(struct hci_request *req)
hci_req_add(req, HCI_OP_LE_SET_ADV_DATA, sizeof(cp), &cp);
}

+static inline u16 eir_append_data(u8 *eir, u16 eir_len, u8 type, u8 *data,
+ u8 data_len)
+{
+ eir[eir_len++] = sizeof(type) + data_len;
+ eir[eir_len++] = type;
+ memcpy(&eir[eir_len], data, data_len);
+ eir_len += data_len;
+
+ return eir_len;
+}
+
static void create_eir(struct hci_dev *hdev, u8 *data)
{
u8 *ptr = data;
@@ -5105,17 +5116,6 @@ void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
mgmt_event(MGMT_EV_NEW_CSRK, hdev, &ev, sizeof(ev), NULL);
}

-static inline u16 eir_append_data(u8 *eir, u16 eir_len, u8 type, u8 *data,
- u8 data_len)
-{
- eir[eir_len++] = sizeof(type) + data_len;
- eir[eir_len++] = type;
- memcpy(&eir[eir_len], data, data_len);
- eir_len += data_len;
-
- return eir_len;
-}
-
void mgmt_device_connected(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
u8 addr_type, u32 flags, u8 *name, u8 name_len,
u8 *dev_class)
--
1.9.2

2014-05-09 19:35:28

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 1/6] Bluetooth: Store TX power level for connection

This patch adds support to store local TX power level for connection
when reply for HCI_Read_Transmit_Power_Level is received.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
---
include/net/bluetooth/hci.h | 11 +++++++++++
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_conn.c | 1 +
net/bluetooth/hci_event.c | 29 +++++++++++++++++++++++++++++
4 files changed, 42 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index ad2ecc9..201a09a 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1054,6 +1054,17 @@ struct hci_cp_write_page_scan_activity {
__le16 window;
} __packed;

+#define HCI_OP_READ_TX_POWER_LEVEL 0x0c2d
+struct hci_cp_read_tx_power_level {
+ __le16 handle;
+ __u8 type;
+} __packed;
+struct hci_rp_read_tx_power_level {
+ __u8 status;
+ __le16 handle;
+ __s8 tx_power_level;
+} __packed;
+
#define HCI_OP_READ_PAGE_SCAN_TYPE 0x0c46
struct hci_rp_read_page_scan_type {
__u8 status;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 0318d52..211bad6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -375,6 +375,7 @@ struct hci_conn {
__u16 le_conn_min_interval;
__u16 le_conn_max_interval;
__s8 rssi;
+ __s8 tx_power;
unsigned long flags;

__u8 remote_cap;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 55a1743..74b368b 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -407,6 +407,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
conn->io_capability = hdev->io_capability;
conn->remote_auth = 0xff;
conn->key_type = 0xff;
+ conn->tx_power = HCI_TX_POWER_INVALID;

set_bit(HCI_CONN_POWER_SAVE, &conn->flags);
conn->disc_timeout = HCI_DISCONN_TIMEOUT;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 2bb0053..63cc9bc 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1264,6 +1264,31 @@ static void hci_cc_read_rssi(struct hci_dev *hdev, struct sk_buff *skb)
hci_dev_unlock(hdev);
}

+static void hci_cc_read_tx_power_level(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_cp_read_tx_power_level *sent;
+ struct hci_rp_read_tx_power_level *rp = (void *) skb->data;
+ struct hci_conn *conn;
+
+ BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
+
+ if (rp->status)
+ return;
+
+ sent = hci_sent_cmd_data(hdev, HCI_OP_READ_TX_POWER_LEVEL);
+ if (!sent)
+ return;
+
+ hci_dev_lock(hdev);
+
+ conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(rp->handle));
+ if (conn && sent->type == 0x00)
+ conn->tx_power = rp->tx_power_level;
+
+ hci_dev_unlock(hdev);
+}
+
static void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
{
BT_DBG("%s status 0x%2.2x", hdev->name, status);
@@ -2660,6 +2685,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
hci_cc_read_rssi(hdev, skb);
break;

+ case HCI_OP_READ_TX_POWER_LEVEL:
+ hci_cc_read_tx_power_level(hdev, skb);
+ break;
+
default:
BT_DBG("%s opcode 0x%4.4x", hdev->name, opcode);
break;
--
1.9.2

2014-05-09 19:35:31

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 4/6] Bluetooth: Make max age for connection information configurable

This patch adds debugfs entry to configure max age of connection
information. Cached values will be only refreshed by Get Connection
Information request if cache is older than this value. Default value
is 2000ms.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
---
include/net/bluetooth/hci_core.h | 4 ++++
net/bluetooth/hci_core.c | 4 ++++
net/bluetooth/mgmt.c | 4 +---
3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 9be523a..6353617 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -145,6 +145,9 @@ struct oob_data {
/* Default LE RPA expiry time, 15 minutes */
#define HCI_DEFAULT_RPA_TIMEOUT (15 * 60)

+/* Default max age of connection information, 2 sec */
+#define DEFAULT_CONN_INFO_MAX_AGE 2000
+
struct amp_assoc {
__u16 len;
__u16 offset;
@@ -200,6 +203,7 @@ struct hci_dev {
__u16 le_conn_min_interval;
__u16 le_conn_max_interval;
__u16 discov_interleaved_timeout;
+ __u16 conn_info_max_age;
__u8 ssp_debug_mode;

__u16 devid_source;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index d31f144..b2d1eee1e3 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1754,6 +1754,9 @@ static int __hci_init(struct hci_dev *hdev)
&blacklist_fops);
debugfs_create_file("uuids", 0444, hdev->debugfs, hdev, &uuids_fops);

+ debugfs_create_u16("conn_info_max_age", 0644, hdev->debugfs,
+ &hdev->conn_info_max_age);
+
if (lmp_bredr_capable(hdev)) {
debugfs_create_file("inquiry_cache", 0444, hdev->debugfs,
hdev, &inquiry_cache_fops);
@@ -3789,6 +3792,7 @@ struct hci_dev *hci_alloc_dev(void)

hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
hdev->discov_interleaved_timeout = DISCOV_INTERLEAVED_TIMEOUT;
+ hdev->conn_info_max_age = DEFAULT_CONN_INFO_MAX_AGE;

mutex_init(&hdev->lock);
mutex_init(&hdev->req_lock);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 2d86ce1..a7f45d2 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -114,8 +114,6 @@ static const u16 mgmt_events[] = {

#define CACHE_TIMEOUT msecs_to_jiffies(2 * 1000)

-#define CONN_INFO_MAX_AGE msecs_to_jiffies(2 * 1000)
-
#define hdev_is_powered(hdev) (test_bit(HCI_UP, &hdev->flags) && \
!test_bit(HCI_AUTO_OFF, &hdev->dev_flags))

@@ -4751,7 +4749,7 @@ static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,
* unknown, otherwise just reply.
*/
if (time_after(jiffies, conn->conn_info_timestamp +
- CONN_INFO_MAX_AGE) ||
+ msecs_to_jiffies(hdev->conn_info_max_age)) ||
conn->tx_power == HCI_TX_POWER_INVALID) {
struct hci_cp_read_tx_power_level req_txp_cp;
struct hci_cp_read_rssi req_rssi_cp;
--
1.9.2

2014-05-09 19:35:30

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 3/6] Bluetooth: Add support to get connection information

This patch adds support for Get Connection Information mgmt command
which can be used to query for information about connection, i.e. RSSI
or local TX power level.

RSSI is returned as response parameter while any other information is
returned in standard EIR format.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 +
include/net/bluetooth/mgmt.h | 16 +++
net/bluetooth/mgmt.c | 216 +++++++++++++++++++++++++++++++++++++++
3 files changed, 234 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 211bad6..9be523a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -378,6 +378,8 @@ struct hci_conn {
__s8 tx_power;
unsigned long flags;

+ unsigned long conn_info_timestamp;
+
__u8 remote_cap;
__u8 remote_auth;
__u8 remote_id;
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index d4b571c..34faa2e 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -409,6 +409,22 @@ struct mgmt_cp_load_irks {
} __packed;
#define MGMT_LOAD_IRKS_SIZE 2

+#define MGMT_CONN_INFO_DATA_TX_POWER 0x00000001
+
+#define MGMT_OP_GET_CONN_INFO 0x0031
+struct mgmt_cp_get_conn_info {
+ struct mgmt_addr_info addr;
+ __u32 data_type;
+} __packed;
+#define MGMT_GET_CONN_INFO_SIZE (MGMT_ADDR_INFO_SIZE + 4)
+struct mgmt_rp_get_conn_info {
+ struct mgmt_addr_info addr;
+ __s8 rssi;
+ __u32 flags;
+ __le16 eir_len;
+ __u8 eir[0];
+} __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 5e88ac9..2d86ce1 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -83,6 +83,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_SET_DEBUG_KEYS,
MGMT_OP_SET_PRIVACY,
MGMT_OP_LOAD_IRKS,
+ MGMT_OP_GET_CONN_INFO,
};

static const u16 mgmt_events[] = {
@@ -113,6 +114,8 @@ static const u16 mgmt_events[] = {

#define CACHE_TIMEOUT msecs_to_jiffies(2 * 1000)

+#define CONN_INFO_MAX_AGE msecs_to_jiffies(2 * 1000)
+
#define hdev_is_powered(hdev) (test_bit(HCI_UP, &hdev->flags) && \
!test_bit(HCI_AUTO_OFF, &hdev->dev_flags))

@@ -4568,6 +4571,218 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
return err;
}

+struct cmd_conn_lookup {
+ struct hci_conn *conn;
+ u8 mgmt_status;
+};
+
+static void get_conn_info_complete(struct pending_cmd *cmd, void *data)
+{
+ struct cmd_conn_lookup *match = data;
+ struct mgmt_cp_get_conn_info *cp;
+ struct mgmt_rp_get_conn_info *rp;
+ char buf[sizeof(*rp) + 3];
+ struct hci_conn *conn = cmd->user_data;
+ __u16 eir_len = 0;
+
+ if (conn != match->conn)
+ return;
+
+ cp = (struct mgmt_cp_get_conn_info *) cmd->param;
+ rp = (struct mgmt_rp_get_conn_info *) buf;
+
+ memset(buf, 0, sizeof(buf));
+ bacpy(&rp->addr.bdaddr, &cp->addr.bdaddr);
+ rp->addr.type = cp->addr.type;
+
+ if (match->mgmt_status) {
+ cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO,
+ match->mgmt_status, rp, sizeof(*rp));
+ goto done;
+ }
+
+ rp->rssi = conn->rssi;
+
+ if (cp->data_type & MGMT_CONN_INFO_DATA_TX_POWER) {
+ eir_len = eir_append_data(rp->eir, 0, EIR_TX_POWER,
+ &conn->tx_power,
+ sizeof(conn->tx_power));
+
+ rp->eir_len = cpu_to_le16(eir_len);
+ }
+
+ cmd_complete(cmd->sk, cmd->index, MGMT_OP_GET_CONN_INFO,
+ MGMT_STATUS_SUCCESS, rp, sizeof(*rp) + eir_len);
+
+done:
+ hci_conn_drop(conn);
+
+ mgmt_pending_remove(cmd);
+}
+
+static void conn_info_refresh_complete(struct hci_dev *hdev, u8 status)
+{
+ struct hci_cp_read_rssi *cp;
+ struct hci_conn *conn;
+ struct cmd_conn_lookup match;
+ u16 handle;
+
+ BT_DBG("status 0x%02x", status);
+
+ hci_dev_lock(hdev);
+
+ /* Last command sent in request should be Read RSSI, but if it isn't
+ * then we look for Read Transmit Power Level which is other command
+ * sent in request and probably failed.
+ *
+ * Both commands have handle as first parameter so it's safe to cast
+ * data on the same command struct.
+ */
+ cp = hci_sent_cmd_data(hdev, HCI_OP_READ_RSSI);
+ if (!cp)
+ cp = hci_sent_cmd_data(hdev, HCI_OP_READ_TX_POWER_LEVEL);
+
+ if (!cp) {
+ BT_ERR("invalid sent_cmd in response");
+ goto unlock;
+ }
+
+ handle = __le16_to_cpu(cp->handle);
+ conn = hci_conn_hash_lookup_handle(hdev, handle);
+ if (!conn) {
+ BT_ERR("unknown handle (%d) in response", handle);
+ goto unlock;
+ }
+
+ if (!status)
+ conn->conn_info_timestamp = jiffies;
+
+ match.conn = conn;
+ match.mgmt_status = mgmt_status(status);
+
+ /* Cache refresh is complete, now reply for each pending request for
+ * given connection.
+ */
+ mgmt_pending_foreach(MGMT_OP_GET_CONN_INFO, hdev,
+ get_conn_info_complete, &match);
+
+unlock:
+ hci_dev_unlock(hdev);
+}
+
+static bool check_pending_conn_info(struct hci_dev *hdev, struct hci_conn *conn)
+{
+ struct pending_cmd *cmd;
+
+ list_for_each_entry(cmd, &hdev->mgmt_pending, list) {
+ if (cmd->opcode == MGMT_OP_GET_CONN_INFO &&
+ cmd->user_data == conn)
+ return true;
+ }
+
+ return false;
+}
+
+static int get_conn_info(struct sock *sk, struct hci_dev *hdev, void *data,
+ u16 len)
+{
+ struct mgmt_cp_get_conn_info *cp = data;
+ struct mgmt_rp_get_conn_info rp;
+ struct hci_conn *conn;
+ struct pending_cmd *cmd;
+ struct hci_request req;
+ bool is_pending;
+ int err = 0;
+
+ BT_DBG("%s", hdev->name);
+
+ memset(&rp, 0, sizeof(rp));
+ bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
+ rp.addr.type = cp->addr.type;
+
+ if (!bdaddr_type_is_valid(cp->addr.type))
+ return cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
+ MGMT_STATUS_INVALID_PARAMS,
+ &rp, sizeof(rp));
+
+ hci_dev_lock(hdev);
+
+ if (!hdev_is_powered(hdev)) {
+ err = cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
+ MGMT_STATUS_NOT_POWERED, &rp, sizeof(rp));
+ goto unlock;
+ }
+
+ if (cp->addr.type == BDADDR_BREDR)
+ conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,
+ &cp->addr.bdaddr);
+ else
+ conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);
+
+ if (!conn || conn->state != BT_CONNECTED) {
+ err = cmd_complete(sk, hdev->id, MGMT_OP_GET_CONN_INFO,
+ MGMT_STATUS_NOT_CONNECTED, &rp, sizeof(rp));
+ goto unlock;
+ }
+
+ /* We need to check if there's another pending request for this conn
+ * so we don't query controller again - just add another pending and
+ * return.
+ */
+ is_pending = check_pending_conn_info(hdev, conn);
+
+ cmd = mgmt_pending_add(sk, MGMT_OP_GET_CONN_INFO, hdev,
+ data, len);
+ if (!cmd) {
+ err = -ENOMEM;
+ goto unlock;
+ }
+
+ hci_conn_hold(conn);
+ cmd->user_data = conn;
+
+ /* There was already pending request for this connection, no need to
+ * start another request to controller.
+ */
+ if (is_pending)
+ goto unlock;
+
+ /* Query controller to refresh cached values if they are too old or
+ * unknown, otherwise just reply.
+ */
+ if (time_after(jiffies, conn->conn_info_timestamp +
+ CONN_INFO_MAX_AGE) ||
+ conn->tx_power == HCI_TX_POWER_INVALID) {
+ struct hci_cp_read_tx_power_level req_txp_cp;
+ struct hci_cp_read_rssi req_rssi_cp;
+
+ hci_req_init(&req, hdev);
+
+ req_txp_cp.handle = cpu_to_le16(conn->handle);
+ req_txp_cp.type = 0x00;
+ hci_req_add(&req, HCI_OP_READ_TX_POWER_LEVEL,
+ sizeof(req_txp_cp), &req_txp_cp);
+
+ req_rssi_cp.handle = cpu_to_le16(conn->handle);
+ hci_req_add(&req, HCI_OP_READ_RSSI, sizeof(req_rssi_cp),
+ &req_rssi_cp);
+
+ err = hci_req_run(&req, conn_info_refresh_complete);
+ if (err < 0) {
+ hci_conn_drop(conn);
+ mgmt_pending_remove(cmd);
+ }
+ } else {
+ struct cmd_conn_lookup match = { conn, MGMT_STATUS_SUCCESS };
+
+ get_conn_info_complete(cmd, &match);
+ }
+
+unlock:
+ hci_dev_unlock(hdev);
+ return err;
+}
+
static const struct mgmt_handler {
int (*func) (struct sock *sk, struct hci_dev *hdev, void *data,
u16 data_len);
@@ -4623,6 +4838,7 @@ static const struct mgmt_handler {
{ set_debug_keys, false, MGMT_SETTING_SIZE },
{ set_privacy, false, MGMT_SET_PRIVACY_SIZE },
{ load_irks, true, MGMT_LOAD_IRKS_SIZE },
+ { get_conn_info, false, MGMT_GET_CONN_INFO_SIZE },
};


--
1.9.2