These changes should improve applications who want to update the connection
parameters for both master and slave devices.
Note: Connection Parameters Request Link Layer Control Procedure (>=4.1) is
not supported in BlueZ yet. I will work on this later as this RFC is approved.
Changes from v3:
* Remove role check in refactored function
* Added handler for Connection Parameter Update Response
* Use Update Request when updating the connection parameters in slave
* Fix MGMT command name to ADD instead of UPDATE
Changes from v2:
* Added new MGMT command
* Roll back to first socket option implementation, details are on the patch
iself.
Changes from v1:
* Use simpler user-space API
* Added patch #1
Felipe F. Tonello (4):
Bluetooth: L2CAP: Refactor L2CAP_CONN_PARAM_UPDATE_REQ into a function
Bluetooth: L2CAP: Add handler for Connection Parameter Update Response
Bluetooth: L2CAP: Add BT_LE_CONN_PARAM socket option
Bluetooth: mgmt: Added new MGMT_OP_ADD_CONN_PARAM command
include/net/bluetooth/bluetooth.h | 8 +++
include/net/bluetooth/l2cap.h | 5 ++
include/net/bluetooth/mgmt.h | 10 ++++
net/bluetooth/l2cap_core.c | 76 +++++++++++++++++++++++----
net/bluetooth/l2cap_sock.c | 106 ++++++++++++++++++++++++++++++++++++++
net/bluetooth/mgmt.c | 103 +++++++++++++++++++++++-------------
6 files changed, 261 insertions(+), 47 deletions(-)
--
2.12.0
Hi Luiz,
On 17/03/17 17:44, Luiz Augusto von Dentz wrote:
> Hi Felipe,
>
> On Tue, Mar 14, 2017 at 8:30 PM, Felipe F. Tonello <[email protected]> wrote:
>> This command is similar to MGMT_OP_LOAD_CONN_PARAM with the exception
>> that it only updates one connection parameter and it doesn't cleanup
>> previous parameters set.
>>
>> This is useful for applications to update one specific connection
>> parameter and not caring about other cached connection parameters.
>>
>> Signed-off-by: Felipe F. Tonello <[email protected]>
>> ---
>> include/net/bluetooth/mgmt.h | 10 +++++
>> net/bluetooth/mgmt.c | 103 ++++++++++++++++++++++++++++---------------
>> 2 files changed, 77 insertions(+), 36 deletions(-)
>>
>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
>> index 72a456bbbcd5..34e5fae8c9d5 100644
>> --- a/include/net/bluetooth/mgmt.h
>> +++ b/include/net/bluetooth/mgmt.h
>> @@ -604,6 +604,16 @@ struct mgmt_cp_set_appearance {
>> } __packed;
>> #define MGMT_SET_APPEARANCE_SIZE 2
>>
>> +#define MGMT_OP_ADD_CONN_PARAM 0x0044
>> +struct mgmt_cp_add_conn_param {
>> + struct mgmt_addr_info addr;
>> + __le16 min_interval;
>> + __le16 max_interval;
>> + __le16 latency;
>> + __le16 timeout;
>> +} __packed;
>> +#define MGMT_ADD_CONN_PARAM_SIZE (MGMT_ADDR_INFO_SIZE + 8)
>
> I do remember mentioning that this command shall accept multiple
> setting at time, just like load.
Yes, that is correct. But as I mentioned on the previous email I sent, I
think this is not the correct approach, since this command effectively
would be only used in the case of peripheral using a Slave Connection
Interval AD. In this case, to avoid multiple calls to this command, I
believe we should parse this particular AD in the Kernel, and just
update the connection parameters accordingly. There is no need to let
user-space know about this, since it is in the AD anyway (it will be
persistent by the slave peripheral wishes).
>
>> +
>> #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 1fba2a03f8ae..b8e86bf1fe1b 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -106,6 +106,7 @@ static const u16 mgmt_commands[] = {
>> MGMT_OP_START_LIMITED_DISCOVERY,
>> MGMT_OP_READ_EXT_INFO,
>> MGMT_OP_SET_APPEARANCE,
>> + MGMT_OP_ADD_CONN_PARAM,
>> };
>>
>> static const u16 mgmt_events[] = {
>> @@ -5462,6 +5463,69 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev,
>> return err;
>> }
>>
>> +/* This function requires the caller holds hdev->lock */
>> +static u8 really_add_conn_param(struct hci_dev *hdev,
>> + struct mgmt_cp_add_conn_param *param)
>> +{
>> + struct hci_conn_params *hci_param;
>> + u16 min, max, latency, timeout;
>> + u8 addr_type;
>> +
>> + if (param->addr.type == BDADDR_LE_PUBLIC)
>> + addr_type = ADDR_LE_DEV_PUBLIC;
>> + else if (param->addr.type == BDADDR_LE_RANDOM)
>> + addr_type = ADDR_LE_DEV_RANDOM;
>> + else
>> + return MGMT_STATUS_INVALID_PARAMS;
>> +
>> + min = le16_to_cpu(param->min_interval);
>> + max = le16_to_cpu(param->max_interval);
>> + latency = le16_to_cpu(param->latency);
>> + timeout = le16_to_cpu(param->timeout);
>> +
>> + BT_DBG("min 0x%04x max 0x%04x latency 0x%04x timeout 0x%04x",
>> + min, max, latency, timeout);
>> +
>> + if (hci_check_conn_params(min, max, latency, timeout) < 0) {
>> + BT_ERR("Ignoring invalid connection parameters");
>> + return MGMT_STATUS_INVALID_PARAMS;
>> + }
>> +
>> + hci_param = hci_conn_params_add(hdev, ¶m->addr.bdaddr,
>> + addr_type);
>> + if (!hci_param) {
>> + BT_ERR("Failed to add connection parameters");
>> + return MGMT_STATUS_FAILED;
>> + }
>> +
>> + hci_param->conn_min_interval = min;
>> + hci_param->conn_max_interval = max;
>> + hci_param->conn_latency = latency;
>> + hci_param->supervision_timeout = timeout;
>> +
>> + return 0;
>> +}
>> +
>> +static int add_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
>> + u16 len)
>> +{
>> + struct mgmt_cp_add_conn_param *cp = data;
>> + u8 cmd_status;
>> +
>> + if (!lmp_le_capable(hdev))
>> + return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_CONN_PARAM,
>> + MGMT_STATUS_NOT_SUPPORTED);
>> +
>> + hci_dev_lock(hdev);
>> +
>> + cmd_status = really_add_conn_param(hdev, cp);
>> +
>> + hci_dev_unlock(hdev);
>> +
>> + return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_CONN_PARAM,
>> + cmd_status, NULL, 0);
>> +}
>> +
>> static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
>> u16 len)
>> {
>> @@ -5500,46 +5564,12 @@ static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
>>
>> for (i = 0; i < param_count; i++) {
>> struct mgmt_conn_param *param = &cp->params[i];
>> - struct hci_conn_params *hci_param;
>> - u16 min, max, latency, timeout;
>> - u8 addr_type;
>>
>> BT_DBG("Adding %pMR (type %u)", ¶m->addr.bdaddr,
>> param->addr.type);
>>
>> - if (param->addr.type == BDADDR_LE_PUBLIC) {
>> - addr_type = ADDR_LE_DEV_PUBLIC;
>> - } else if (param->addr.type == BDADDR_LE_RANDOM) {
>> - addr_type = ADDR_LE_DEV_RANDOM;
>> - } else {
>> - BT_ERR("Ignoring invalid connection parameters");
>> - continue;
>> - }
>> -
>> - min = le16_to_cpu(param->min_interval);
>> - max = le16_to_cpu(param->max_interval);
>> - latency = le16_to_cpu(param->latency);
>> - timeout = le16_to_cpu(param->timeout);
>> -
>> - BT_DBG("min 0x%04x max 0x%04x latency 0x%04x timeout 0x%04x",
>> - min, max, latency, timeout);
>> -
>> - if (hci_check_conn_params(min, max, latency, timeout) < 0) {
>> - BT_ERR("Ignoring invalid connection parameters");
>> - continue;
>> - }
>> -
>> - hci_param = hci_conn_params_add(hdev, ¶m->addr.bdaddr,
>> - addr_type);
>> - if (!hci_param) {
>> - BT_ERR("Failed to add connection parameters");
>> - continue;
>> - }
>> -
>> - hci_param->conn_min_interval = min;
>> - hci_param->conn_max_interval = max;
>> - hci_param->conn_latency = latency;
>> - hci_param->supervision_timeout = timeout;
>> + really_add_conn_param(hdev,
>> + (struct mgmt_cp_add_conn_param *)param);
>
> Once Add support more than one setting the only real difference from
> Load should be that Load does reset the settings before adding,
> everything else remains the same.
>
>> }
>>
>> hci_dev_unlock(hdev);
>> @@ -6538,6 +6568,7 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
>> { read_ext_controller_info,MGMT_READ_EXT_INFO_SIZE,
>> HCI_MGMT_UNTRUSTED },
>> { set_appearance, MGMT_SET_APPEARANCE_SIZE },
>> + { add_conn_param, MGMT_ADD_CONN_PARAM_SIZE },
>> };
>>
>> void mgmt_index_added(struct hci_dev *hdev)
>> --
>> 2.12.0
>>
>
--
Felipe
Hi Felipe,
On Tue, Mar 14, 2017 at 8:30 PM, Felipe F. Tonello <[email protected]> wrote:
> This command is similar to MGMT_OP_LOAD_CONN_PARAM with the exception
> that it only updates one connection parameter and it doesn't cleanup
> previous parameters set.
>
> This is useful for applications to update one specific connection
> parameter and not caring about other cached connection parameters.
>
> Signed-off-by: Felipe F. Tonello <[email protected]>
> ---
> include/net/bluetooth/mgmt.h | 10 +++++
> net/bluetooth/mgmt.c | 103 ++++++++++++++++++++++++++++---------------
> 2 files changed, 77 insertions(+), 36 deletions(-)
>
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 72a456bbbcd5..34e5fae8c9d5 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -604,6 +604,16 @@ struct mgmt_cp_set_appearance {
> } __packed;
> #define MGMT_SET_APPEARANCE_SIZE 2
>
> +#define MGMT_OP_ADD_CONN_PARAM 0x0044
> +struct mgmt_cp_add_conn_param {
> + struct mgmt_addr_info addr;
> + __le16 min_interval;
> + __le16 max_interval;
> + __le16 latency;
> + __le16 timeout;
> +} __packed;
> +#define MGMT_ADD_CONN_PARAM_SIZE (MGMT_ADDR_INFO_SIZE + 8)
I do remember mentioning that this command shall accept multiple
setting at time, just like load.
> +
> #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 1fba2a03f8ae..b8e86bf1fe1b 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -106,6 +106,7 @@ static const u16 mgmt_commands[] = {
> MGMT_OP_START_LIMITED_DISCOVERY,
> MGMT_OP_READ_EXT_INFO,
> MGMT_OP_SET_APPEARANCE,
> + MGMT_OP_ADD_CONN_PARAM,
> };
>
> static const u16 mgmt_events[] = {
> @@ -5462,6 +5463,69 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev,
> return err;
> }
>
> +/* This function requires the caller holds hdev->lock */
> +static u8 really_add_conn_param(struct hci_dev *hdev,
> + struct mgmt_cp_add_conn_param *param)
> +{
> + struct hci_conn_params *hci_param;
> + u16 min, max, latency, timeout;
> + u8 addr_type;
> +
> + if (param->addr.type == BDADDR_LE_PUBLIC)
> + addr_type = ADDR_LE_DEV_PUBLIC;
> + else if (param->addr.type == BDADDR_LE_RANDOM)
> + addr_type = ADDR_LE_DEV_RANDOM;
> + else
> + return MGMT_STATUS_INVALID_PARAMS;
> +
> + min = le16_to_cpu(param->min_interval);
> + max = le16_to_cpu(param->max_interval);
> + latency = le16_to_cpu(param->latency);
> + timeout = le16_to_cpu(param->timeout);
> +
> + BT_DBG("min 0x%04x max 0x%04x latency 0x%04x timeout 0x%04x",
> + min, max, latency, timeout);
> +
> + if (hci_check_conn_params(min, max, latency, timeout) < 0) {
> + BT_ERR("Ignoring invalid connection parameters");
> + return MGMT_STATUS_INVALID_PARAMS;
> + }
> +
> + hci_param = hci_conn_params_add(hdev, ¶m->addr.bdaddr,
> + addr_type);
> + if (!hci_param) {
> + BT_ERR("Failed to add connection parameters");
> + return MGMT_STATUS_FAILED;
> + }
> +
> + hci_param->conn_min_interval = min;
> + hci_param->conn_max_interval = max;
> + hci_param->conn_latency = latency;
> + hci_param->supervision_timeout = timeout;
> +
> + return 0;
> +}
> +
> +static int add_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
> + u16 len)
> +{
> + struct mgmt_cp_add_conn_param *cp = data;
> + u8 cmd_status;
> +
> + if (!lmp_le_capable(hdev))
> + return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_CONN_PARAM,
> + MGMT_STATUS_NOT_SUPPORTED);
> +
> + hci_dev_lock(hdev);
> +
> + cmd_status = really_add_conn_param(hdev, cp);
> +
> + hci_dev_unlock(hdev);
> +
> + return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_CONN_PARAM,
> + cmd_status, NULL, 0);
> +}
> +
> static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
> u16 len)
> {
> @@ -5500,46 +5564,12 @@ static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
>
> for (i = 0; i < param_count; i++) {
> struct mgmt_conn_param *param = &cp->params[i];
> - struct hci_conn_params *hci_param;
> - u16 min, max, latency, timeout;
> - u8 addr_type;
>
> BT_DBG("Adding %pMR (type %u)", ¶m->addr.bdaddr,
> param->addr.type);
>
> - if (param->addr.type == BDADDR_LE_PUBLIC) {
> - addr_type = ADDR_LE_DEV_PUBLIC;
> - } else if (param->addr.type == BDADDR_LE_RANDOM) {
> - addr_type = ADDR_LE_DEV_RANDOM;
> - } else {
> - BT_ERR("Ignoring invalid connection parameters");
> - continue;
> - }
> -
> - min = le16_to_cpu(param->min_interval);
> - max = le16_to_cpu(param->max_interval);
> - latency = le16_to_cpu(param->latency);
> - timeout = le16_to_cpu(param->timeout);
> -
> - BT_DBG("min 0x%04x max 0x%04x latency 0x%04x timeout 0x%04x",
> - min, max, latency, timeout);
> -
> - if (hci_check_conn_params(min, max, latency, timeout) < 0) {
> - BT_ERR("Ignoring invalid connection parameters");
> - continue;
> - }
> -
> - hci_param = hci_conn_params_add(hdev, ¶m->addr.bdaddr,
> - addr_type);
> - if (!hci_param) {
> - BT_ERR("Failed to add connection parameters");
> - continue;
> - }
> -
> - hci_param->conn_min_interval = min;
> - hci_param->conn_max_interval = max;
> - hci_param->conn_latency = latency;
> - hci_param->supervision_timeout = timeout;
> + really_add_conn_param(hdev,
> + (struct mgmt_cp_add_conn_param *)param);
Once Add support more than one setting the only real difference from
Load should be that Load does reset the settings before adding,
everything else remains the same.
> }
>
> hci_dev_unlock(hdev);
> @@ -6538,6 +6568,7 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
> { read_ext_controller_info,MGMT_READ_EXT_INFO_SIZE,
> HCI_MGMT_UNTRUSTED },
> { set_appearance, MGMT_SET_APPEARANCE_SIZE },
> + { add_conn_param, MGMT_ADD_CONN_PARAM_SIZE },
> };
>
> void mgmt_index_added(struct hci_dev *hdev)
> --
> 2.12.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
Hi,
On 14/03/17 18:30, Felipe F. Tonello wrote:
> This command is similar to MGMT_OP_LOAD_CONN_PARAM with the exception
> that it only updates one connection parameter and it doesn't cleanup
> previous parameters set.
>
> This is useful for applications to update one specific connection
> parameter and not caring about other cached connection parameters.
TBH, I believe this is the wrong approach. The Slave Connection Interval
should be parsed in the kernel and not in user-space, to avoid loads of
context-switch.
>
> Signed-off-by: Felipe F. Tonello <[email protected]>
> ---
> include/net/bluetooth/mgmt.h | 10 +++++
> net/bluetooth/mgmt.c | 103 ++++++++++++++++++++++++++++---------------
> 2 files changed, 77 insertions(+), 36 deletions(-)
>
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 72a456bbbcd5..34e5fae8c9d5 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -604,6 +604,16 @@ struct mgmt_cp_set_appearance {
> } __packed;
> #define MGMT_SET_APPEARANCE_SIZE 2
>
> +#define MGMT_OP_ADD_CONN_PARAM 0x0044
> +struct mgmt_cp_add_conn_param {
> + struct mgmt_addr_info addr;
> + __le16 min_interval;
> + __le16 max_interval;
> + __le16 latency;
> + __le16 timeout;
> +} __packed;
> +#define MGMT_ADD_CONN_PARAM_SIZE (MGMT_ADDR_INFO_SIZE + 8)
> +
> #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 1fba2a03f8ae..b8e86bf1fe1b 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -106,6 +106,7 @@ static const u16 mgmt_commands[] = {
> MGMT_OP_START_LIMITED_DISCOVERY,
> MGMT_OP_READ_EXT_INFO,
> MGMT_OP_SET_APPEARANCE,
> + MGMT_OP_ADD_CONN_PARAM,
> };
>
> static const u16 mgmt_events[] = {
> @@ -5462,6 +5463,69 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev,
> return err;
> }
>
> +/* This function requires the caller holds hdev->lock */
> +static u8 really_add_conn_param(struct hci_dev *hdev,
> + struct mgmt_cp_add_conn_param *param)
> +{
> + struct hci_conn_params *hci_param;
> + u16 min, max, latency, timeout;
> + u8 addr_type;
> +
> + if (param->addr.type == BDADDR_LE_PUBLIC)
> + addr_type = ADDR_LE_DEV_PUBLIC;
> + else if (param->addr.type == BDADDR_LE_RANDOM)
> + addr_type = ADDR_LE_DEV_RANDOM;
> + else
> + return MGMT_STATUS_INVALID_PARAMS;
> +
> + min = le16_to_cpu(param->min_interval);
> + max = le16_to_cpu(param->max_interval);
> + latency = le16_to_cpu(param->latency);
> + timeout = le16_to_cpu(param->timeout);
> +
> + BT_DBG("min 0x%04x max 0x%04x latency 0x%04x timeout 0x%04x",
> + min, max, latency, timeout);
> +
> + if (hci_check_conn_params(min, max, latency, timeout) < 0) {
> + BT_ERR("Ignoring invalid connection parameters");
> + return MGMT_STATUS_INVALID_PARAMS;
> + }
> +
> + hci_param = hci_conn_params_add(hdev, ¶m->addr.bdaddr,
> + addr_type);
> + if (!hci_param) {
> + BT_ERR("Failed to add connection parameters");
> + return MGMT_STATUS_FAILED;
> + }
> +
> + hci_param->conn_min_interval = min;
> + hci_param->conn_max_interval = max;
> + hci_param->conn_latency = latency;
> + hci_param->supervision_timeout = timeout;
> +
> + return 0;
> +}
> +
> +static int add_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
> + u16 len)
> +{
> + struct mgmt_cp_add_conn_param *cp = data;
> + u8 cmd_status;
> +
> + if (!lmp_le_capable(hdev))
> + return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_CONN_PARAM,
> + MGMT_STATUS_NOT_SUPPORTED);
> +
> + hci_dev_lock(hdev);
> +
> + cmd_status = really_add_conn_param(hdev, cp);
> +
> + hci_dev_unlock(hdev);
> +
> + return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_CONN_PARAM,
> + cmd_status, NULL, 0);
> +}
> +
> static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
> u16 len)
> {
> @@ -5500,46 +5564,12 @@ static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
>
> for (i = 0; i < param_count; i++) {
> struct mgmt_conn_param *param = &cp->params[i];
> - struct hci_conn_params *hci_param;
> - u16 min, max, latency, timeout;
> - u8 addr_type;
>
> BT_DBG("Adding %pMR (type %u)", ¶m->addr.bdaddr,
> param->addr.type);
>
> - if (param->addr.type == BDADDR_LE_PUBLIC) {
> - addr_type = ADDR_LE_DEV_PUBLIC;
> - } else if (param->addr.type == BDADDR_LE_RANDOM) {
> - addr_type = ADDR_LE_DEV_RANDOM;
> - } else {
> - BT_ERR("Ignoring invalid connection parameters");
> - continue;
> - }
> -
> - min = le16_to_cpu(param->min_interval);
> - max = le16_to_cpu(param->max_interval);
> - latency = le16_to_cpu(param->latency);
> - timeout = le16_to_cpu(param->timeout);
> -
> - BT_DBG("min 0x%04x max 0x%04x latency 0x%04x timeout 0x%04x",
> - min, max, latency, timeout);
> -
> - if (hci_check_conn_params(min, max, latency, timeout) < 0) {
> - BT_ERR("Ignoring invalid connection parameters");
> - continue;
> - }
> -
> - hci_param = hci_conn_params_add(hdev, ¶m->addr.bdaddr,
> - addr_type);
> - if (!hci_param) {
> - BT_ERR("Failed to add connection parameters");
> - continue;
> - }
> -
> - hci_param->conn_min_interval = min;
> - hci_param->conn_max_interval = max;
> - hci_param->conn_latency = latency;
> - hci_param->supervision_timeout = timeout;
> + really_add_conn_param(hdev,
> + (struct mgmt_cp_add_conn_param *)param);
> }
>
> hci_dev_unlock(hdev);
> @@ -6538,6 +6568,7 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
> { read_ext_controller_info,MGMT_READ_EXT_INFO_SIZE,
> HCI_MGMT_UNTRUSTED },
> { set_appearance, MGMT_SET_APPEARANCE_SIZE },
> + { add_conn_param, MGMT_ADD_CONN_PARAM_SIZE },
> };
>
> void mgmt_index_added(struct hci_dev *hdev)
>
--
Felipe
In case of connected hosts that doesn't support the Connection
Parameters Request Link Layer Control Procedure, we should support this
event response from a slave device.
The slave device will answer based on a previous request done by the
master (BlueZ in this case) and update current connection parameters
accordingly. If the request was rejected, we don't do anything.
Signed-off-by: Felipe F. Tonello <[email protected]>
---
include/net/bluetooth/l2cap.h | 2 ++
net/bluetooth/l2cap_core.c | 45 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 0073a01c61e9..eb58d811596f 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -654,6 +654,8 @@ struct l2cap_conn {
struct mutex chan_lock;
struct kref ref;
struct list_head users;
+
+ struct l2cap_conn_param_update_req conn_param_req;
};
struct l2cap_user {
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 27a38b4543e1..b7c34eb4e912 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1489,6 +1489,13 @@ void l2cap_le_conn_update_req(struct l2cap_conn *conn, u8 min_interval,
{
struct l2cap_conn_param_update_req req;
+ /* set temporary parameters in case of a successful
+ * response from the peer device */
+ conn->conn_param_req.min = min_interval;
+ conn->conn_param_req.max = max_interval;
+ conn->conn_param_req.latency = latency;
+ conn->conn_param_req.to_multiplier = supv_timeout;
+
req.min = cpu_to_le16(min_interval);
req.max = cpu_to_le16(max_interval);
req.latency = cpu_to_le16(latency);
@@ -5251,6 +5258,43 @@ static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
return 0;
}
+static inline int l2cap_conn_param_update_rsp(struct l2cap_conn *conn,
+ struct l2cap_cmd_hdr *cmd,
+ u16 cmd_len, u8 *data)
+{
+ struct hci_conn *hcon = conn->hcon;
+ struct l2cap_conn_param_update_rsp *rsp;
+ u8 result;
+
+ if (hcon->role != HCI_ROLE_SLAVE)
+ return -EINVAL;
+
+ if (cmd_len != sizeof(struct l2cap_conn_param_update_rsp))
+ return -EPROTO;
+
+ rsp = (struct l2cap_conn_param_update_rsp *) data;
+ result = le16_to_cpu(rsp->result);
+
+ BT_DBG("result 0x%4.4x", result);
+
+ if (result == 0) {
+ u8 store_hint;
+
+ store_hint = hci_le_conn_update(hcon,
+ conn->conn_param_req.min,
+ conn->conn_param_req.max,
+ conn->conn_param_req.latency,
+ conn->conn_param_req.to_multiplier);
+ mgmt_new_conn_param(hcon->hdev, &hcon->dst, hcon->dst_type,
+ store_hint, conn->conn_param_req.min,
+ conn->conn_param_req.max,
+ conn->conn_param_req.latency,
+ conn->conn_param_req.to_multiplier);
+ }
+
+ return 0;
+}
+
static int l2cap_le_connect_rsp(struct l2cap_conn *conn,
struct l2cap_cmd_hdr *cmd, u16 cmd_len,
u8 *data)
@@ -5633,6 +5677,7 @@ static inline int l2cap_le_sig_cmd(struct l2cap_conn *conn,
break;
case L2CAP_CONN_PARAM_UPDATE_RSP:
+ err = l2cap_conn_param_update_rsp(conn, cmd, cmd_len, data);
break;
case L2CAP_LE_CONN_RSP:
--
2.12.0
This command is similar to MGMT_OP_LOAD_CONN_PARAM with the exception
that it only updates one connection parameter and it doesn't cleanup
previous parameters set.
This is useful for applications to update one specific connection
parameter and not caring about other cached connection parameters.
Signed-off-by: Felipe F. Tonello <[email protected]>
---
include/net/bluetooth/mgmt.h | 10 +++++
net/bluetooth/mgmt.c | 103 ++++++++++++++++++++++++++++---------------
2 files changed, 77 insertions(+), 36 deletions(-)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 72a456bbbcd5..34e5fae8c9d5 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -604,6 +604,16 @@ struct mgmt_cp_set_appearance {
} __packed;
#define MGMT_SET_APPEARANCE_SIZE 2
+#define MGMT_OP_ADD_CONN_PARAM 0x0044
+struct mgmt_cp_add_conn_param {
+ struct mgmt_addr_info addr;
+ __le16 min_interval;
+ __le16 max_interval;
+ __le16 latency;
+ __le16 timeout;
+} __packed;
+#define MGMT_ADD_CONN_PARAM_SIZE (MGMT_ADDR_INFO_SIZE + 8)
+
#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 1fba2a03f8ae..b8e86bf1fe1b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -106,6 +106,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_START_LIMITED_DISCOVERY,
MGMT_OP_READ_EXT_INFO,
MGMT_OP_SET_APPEARANCE,
+ MGMT_OP_ADD_CONN_PARAM,
};
static const u16 mgmt_events[] = {
@@ -5462,6 +5463,69 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev,
return err;
}
+/* This function requires the caller holds hdev->lock */
+static u8 really_add_conn_param(struct hci_dev *hdev,
+ struct mgmt_cp_add_conn_param *param)
+{
+ struct hci_conn_params *hci_param;
+ u16 min, max, latency, timeout;
+ u8 addr_type;
+
+ if (param->addr.type == BDADDR_LE_PUBLIC)
+ addr_type = ADDR_LE_DEV_PUBLIC;
+ else if (param->addr.type == BDADDR_LE_RANDOM)
+ addr_type = ADDR_LE_DEV_RANDOM;
+ else
+ return MGMT_STATUS_INVALID_PARAMS;
+
+ min = le16_to_cpu(param->min_interval);
+ max = le16_to_cpu(param->max_interval);
+ latency = le16_to_cpu(param->latency);
+ timeout = le16_to_cpu(param->timeout);
+
+ BT_DBG("min 0x%04x max 0x%04x latency 0x%04x timeout 0x%04x",
+ min, max, latency, timeout);
+
+ if (hci_check_conn_params(min, max, latency, timeout) < 0) {
+ BT_ERR("Ignoring invalid connection parameters");
+ return MGMT_STATUS_INVALID_PARAMS;
+ }
+
+ hci_param = hci_conn_params_add(hdev, ¶m->addr.bdaddr,
+ addr_type);
+ if (!hci_param) {
+ BT_ERR("Failed to add connection parameters");
+ return MGMT_STATUS_FAILED;
+ }
+
+ hci_param->conn_min_interval = min;
+ hci_param->conn_max_interval = max;
+ hci_param->conn_latency = latency;
+ hci_param->supervision_timeout = timeout;
+
+ return 0;
+}
+
+static int add_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
+ u16 len)
+{
+ struct mgmt_cp_add_conn_param *cp = data;
+ u8 cmd_status;
+
+ if (!lmp_le_capable(hdev))
+ return mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_CONN_PARAM,
+ MGMT_STATUS_NOT_SUPPORTED);
+
+ hci_dev_lock(hdev);
+
+ cmd_status = really_add_conn_param(hdev, cp);
+
+ hci_dev_unlock(hdev);
+
+ return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_CONN_PARAM,
+ cmd_status, NULL, 0);
+}
+
static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
u16 len)
{
@@ -5500,46 +5564,12 @@ static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
for (i = 0; i < param_count; i++) {
struct mgmt_conn_param *param = &cp->params[i];
- struct hci_conn_params *hci_param;
- u16 min, max, latency, timeout;
- u8 addr_type;
BT_DBG("Adding %pMR (type %u)", ¶m->addr.bdaddr,
param->addr.type);
- if (param->addr.type == BDADDR_LE_PUBLIC) {
- addr_type = ADDR_LE_DEV_PUBLIC;
- } else if (param->addr.type == BDADDR_LE_RANDOM) {
- addr_type = ADDR_LE_DEV_RANDOM;
- } else {
- BT_ERR("Ignoring invalid connection parameters");
- continue;
- }
-
- min = le16_to_cpu(param->min_interval);
- max = le16_to_cpu(param->max_interval);
- latency = le16_to_cpu(param->latency);
- timeout = le16_to_cpu(param->timeout);
-
- BT_DBG("min 0x%04x max 0x%04x latency 0x%04x timeout 0x%04x",
- min, max, latency, timeout);
-
- if (hci_check_conn_params(min, max, latency, timeout) < 0) {
- BT_ERR("Ignoring invalid connection parameters");
- continue;
- }
-
- hci_param = hci_conn_params_add(hdev, ¶m->addr.bdaddr,
- addr_type);
- if (!hci_param) {
- BT_ERR("Failed to add connection parameters");
- continue;
- }
-
- hci_param->conn_min_interval = min;
- hci_param->conn_max_interval = max;
- hci_param->conn_latency = latency;
- hci_param->supervision_timeout = timeout;
+ really_add_conn_param(hdev,
+ (struct mgmt_cp_add_conn_param *)param);
}
hci_dev_unlock(hdev);
@@ -6538,6 +6568,7 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
{ read_ext_controller_info,MGMT_READ_EXT_INFO_SIZE,
HCI_MGMT_UNTRUSTED },
{ set_appearance, MGMT_SET_APPEARANCE_SIZE },
+ { add_conn_param, MGMT_ADD_CONN_PARAM_SIZE },
};
void mgmt_index_added(struct hci_dev *hdev)
--
2.12.0
There is a need for certain LE profiles (MIDI for example) to change the
current connection's parameters. In order to do that, this patch
introduces a new BT_LE_CONN_PARAM socket option for SOL_BLUETOOTH
protocol which allow user-space l2cap sockets to update the current
connection.
It necessary to expose all the connection parameters to user-space
because user-space are exposed to those values anyway, via PPCP
characteristic or Slave Connection Interval AD, for instance. Also it is
useful for tools to set particular values for because of profile
requirements.
If ROLE is SLAVE, then it will also send a L2CAP_CONN_PARAM_UPDATE_REQ
signaling command to the MASTER, triggering proper 4.0 parameter update
procedure.
This option will also send a MGMT_EV_NEW_CONN_PARAM event with the
store_hint set so the user-space application can store the connection
parameters for persistence reasons.
Note: Connection Parameters Request Link Layer Control Procedure is not
supported by BlueZ yet.
Signed-off-by: Felipe F. Tonello <[email protected]>
---
include/net/bluetooth/bluetooth.h | 8 +++
net/bluetooth/l2cap_sock.c | 106 ++++++++++++++++++++++++++++++++++++++
2 files changed, 114 insertions(+)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 01487192f628..ce5b3abd3b27 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -122,6 +122,14 @@ struct bt_voice {
#define BT_SNDMTU 12
#define BT_RCVMTU 13
+#define BT_LE_CONN_PARAM 14
+struct bt_le_conn_param {
+ __u16 min_interval;
+ __u16 max_interval;
+ __u16 latency;
+ __u16 supervision_timeout;
+};
+
__printf(1, 2)
void bt_info(const char *fmt, ...);
__printf(1, 2)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index f307b145ea54..f1c83e021b08 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -515,6 +515,47 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
lock_sock(sk);
switch (optname) {
+ case BT_LE_CONN_PARAM: {
+ struct bt_le_conn_param conn_param;
+ struct hci_conn_params *params;
+ struct hci_conn *hcon;
+ struct hci_dev *hdev;
+
+ if (!chan->conn) {
+ err = -ENOTCONN;
+ break;
+ }
+
+ hcon = chan->conn->hcon;
+ hdev = hcon->hdev;
+ hci_dev_lock(hdev);
+
+ params = hci_conn_params_lookup(hdev, &hcon->dst,
+ hcon->dst_type);
+
+ memset(&conn_param, 0, sizeof(conn_param));
+
+ if (params) {
+ conn_param.min_interval = params->conn_min_interval;
+ conn_param.max_interval = params->conn_max_interval;
+ conn_param.latency = params->conn_latency;
+ conn_param.supervision_timeout =
+ params->supervision_timeout;
+ } else {
+ conn_param.min_interval = hdev->le_conn_min_interval;
+ conn_param.max_interval = hdev->le_conn_max_interval;
+ conn_param.latency = hdev->le_conn_latency;
+ conn_param.supervision_timeout = hdev->le_supv_timeout;
+ }
+
+ hci_dev_unlock(hdev);
+
+ len = min_t(unsigned int, len, sizeof(conn_param));
+ if (copy_to_user(optval, (char *) &conn_param, len))
+ err = -EFAULT;
+
+ break;
+ }
case BT_SECURITY:
if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED &&
chan->chan_type != L2CAP_CHAN_FIXED &&
@@ -762,6 +803,71 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
lock_sock(sk);
switch (optname) {
+ case BT_LE_CONN_PARAM: {
+ struct bt_le_conn_param conn_param;
+ struct hci_conn_params *hci_param;
+ struct hci_conn *hcon;
+ struct hci_dev *hdev;
+
+ len = min_t(unsigned int, sizeof(conn_param), optlen);
+ if (copy_from_user((char *) &conn_param, optval, len)) {
+ err = -EFAULT;
+ break;
+ }
+
+ if (!chan->conn) {
+ err = -ENOTCONN;
+ break;
+ }
+
+ err = hci_check_conn_params(conn_param.min_interval,
+ conn_param.max_interval, conn_param.latency,
+ conn_param.supervision_timeout);
+ if (err < 0) {
+ BT_ERR("Ignoring invalid connection parameters");
+ break;
+ }
+
+ hcon = chan->conn->hcon;
+ hdev = hcon->hdev;
+
+ hci_dev_lock(hdev);
+
+ /* we add new param in case it doesn't exist */
+ hci_param = hci_conn_params_add(hdev, &hcon->dst,
+ hcon->dst_type);
+ if (!hci_param) {
+ err = -ENOMEM;
+ BT_ERR("Failed to add connection parameters");
+ hci_dev_unlock(hcon->hdev);
+ break;
+ }
+
+ hci_dev_unlock(hdev);
+
+ /* Send a L2CAP connection parameters update request, if
+ * the host role is slave. */
+ if (hcon->role == HCI_ROLE_SLAVE) {
+ l2cap_le_conn_update_req(chan->conn,
+ conn_param.min_interval,
+ conn_param.max_interval,
+ conn_param.latency,
+ conn_param.supervision_timeout);
+ } else {
+ /* this function also updates the hci_param value */
+ hci_le_conn_update(hcon, conn_param.min_interval,
+ conn_param.max_interval, conn_param.latency,
+ conn_param.supervision_timeout);
+
+ /* don't set the `store' hint */
+ mgmt_new_conn_param(hdev, &hcon->dst, hcon->dst_type,
+ false, conn_param.min_interval,
+ conn_param.max_interval, conn_param.latency,
+ conn_param.supervision_timeout);
+ }
+ break;
+ }
+
case BT_SECURITY:
if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED &&
chan->chan_type != L2CAP_CHAN_FIXED &&
--
2.12.0
This signaling command is useful for any connection parameter change
procedure, thus it is important to allow access to it from outside this
translation unit.
Signed-off-by: Felipe F. Tonello <[email protected]>
---
include/net/bluetooth/l2cap.h | 3 +++
net/bluetooth/l2cap_core.c | 31 ++++++++++++++++++++-----------
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 5ee3c689c863..0073a01c61e9 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -948,4 +948,7 @@ void l2cap_conn_put(struct l2cap_conn *conn);
int l2cap_register_user(struct l2cap_conn *conn, struct l2cap_user *user);
void l2cap_unregister_user(struct l2cap_conn *conn, struct l2cap_user *user);
+void l2cap_le_conn_update_req(struct l2cap_conn *conn, u8 min_interval,
+ u8 max_interval, u8 latency, u8 supv_timeout);
+
#endif /* __L2CAP_H */
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index fc7f321a3823..27a38b4543e1 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1483,6 +1483,22 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
mutex_unlock(&conn->chan_lock);
}
+/* This command shall only be send if the connection role is master */
+void l2cap_le_conn_update_req(struct l2cap_conn *conn, u8 min_interval,
+ u8 max_interval, u8 latency, u8 supv_timeout)
+{
+ struct l2cap_conn_param_update_req req;
+
+ req.min = cpu_to_le16(min_interval);
+ req.max = cpu_to_le16(max_interval);
+ req.latency = cpu_to_le16(latency);
+ req.to_multiplier = cpu_to_le16(supv_timeout);
+
+ l2cap_send_cmd(conn, l2cap_get_ident(conn),
+ L2CAP_CONN_PARAM_UPDATE_REQ, sizeof(req), &req);
+}
+
+
static void l2cap_le_conn_ready(struct l2cap_conn *conn)
{
struct hci_conn *hcon = conn->hcon;
@@ -1503,17 +1519,10 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
*/
if (hcon->role == HCI_ROLE_SLAVE &&
(hcon->le_conn_interval < hcon->le_conn_min_interval ||
- hcon->le_conn_interval > hcon->le_conn_max_interval)) {
- struct l2cap_conn_param_update_req req;
-
- req.min = cpu_to_le16(hcon->le_conn_min_interval);
- req.max = cpu_to_le16(hcon->le_conn_max_interval);
- req.latency = cpu_to_le16(hcon->le_conn_latency);
- req.to_multiplier = cpu_to_le16(hcon->le_supv_timeout);
-
- l2cap_send_cmd(conn, l2cap_get_ident(conn),
- L2CAP_CONN_PARAM_UPDATE_REQ, sizeof(req), &req);
- }
+ hcon->le_conn_interval > hcon->le_conn_max_interval))
+ l2cap_le_conn_update_req(conn, hcon->le_conn_min_interval,
+ hcon->le_conn_max_interval, hcon->le_conn_latency,
+ hcon->le_supv_timeout);
}
static void l2cap_conn_ready(struct l2cap_conn *conn)
--
2.12.0