Re: [RFC][PATCH v3 BlueZ 2/3] Bluetooth: L2CAP: Add BT_LE_CONN_PARAM socket option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Luiz,

On 14/03/17 13:17, Luiz Augusto von Dentz wrote:
> Hi Felipe,
> 
> On Mon, Mar 13, 2017 at 7:54 PM, Felipe F. Tonello <eu@xxxxxxxxxxxxxxxxx> wrote:
>> 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 <eu@xxxxxxxxxxxxxxxxx>
>> ---
>>  include/net/bluetooth/bluetooth.h |   8 +++
>>  net/bluetooth/l2cap_sock.c        | 101 ++++++++++++++++++++++++++++++++++++++
>>  net/bluetooth/mgmt.c              |   1 +
>>  3 files changed, 110 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..8c2e6f29869f 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,66 @@ 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 will be updated in hci_le_conn_update(). */
>> +               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. */
>> +               l2cap_le_conn_req(chan->conn, conn_param.min_interval,
>> +                       conn_param.max_interval, conn_param.latency,
>> +                       conn_param.supervision_timeout);
> 
> I thought we already discuss doing something like this should require
> us to wait for the response from the remote, if it doesn't accept the
> new parameters or suggests something else then it needs to be stored
> accordingly. Also perhaps the socket options should only work as
> overwrite values for the current connection and not affect the stored
> values because this may be relevant for only a certain cid/PSM and if
> that is not reconnected there is no reason to reapply the same
> parameters all the time.

Ok.

> 
>> +               /* 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);
>> +
>> +               mgmt_new_conn_param(hdev, &hcon->dst, hcon->dst_type, true,
>> +                       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 &&
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 1fba2a03f8ae..0f44af559ae6 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -5540,6 +5540,7 @@ static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
>>                 hci_param->conn_max_interval = max;
>>                 hci_param->conn_latency = latency;
>>                 hci_param->supervision_timeout = timeout;
>> +               hci_param->conn_config = BT_LE_CONN_CONFIG_CUSTOM_LATENCY;
>>         }
>>
>>         hci_dev_unlock(hdev);
>> --
>> 2.12.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

-- 
Felipe

Attachment: 0x92698E6A.asc
Description: application/pgp-keys


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux