[RFC][PATCH BlueZ] Bluetooth: L2CAP: Add BT_LE_CONN_PARAM socket option

[RFC][PATCH BlueZ] Bluetooth: L2CAP: Add BT_LE_CONN_PARAM socket option

[Felipe F. Tonello]

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.

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.

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
 include/net/bluetooth/bluetooth.h |  8 ++++
 net/bluetooth/l2cap_sock.c        | 98 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 106 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 a8ba752732c9..9c492730b081 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -498,6 +498,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
 	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
 	struct bt_security sec;
 	struct bt_power pwr;
+	struct bt_le_conn_param conn_param;
 	int len, err = 0;
 
 	BT_DBG("sk %p", sk);
@@ -514,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 hci_conn_params *params;
+		struct hci_conn *hcon;
+
+		if (!chan->conn) {
+			err = -ENOTCONN;
+			break;
+		}
+
+		hcon = chan->conn->hcon;
+		hci_dev_lock(hcon->hdev);
+
+		params = hci_conn_params_lookup(hcon->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 =
+				chan->conn->hcon->hdev->le_conn_min_interval;
+			conn_param.max_interval =
+				chan->conn->hcon->hdev->le_conn_max_interval;
+			conn_param.latency =
+				chan->conn->hcon->hdev->le_conn_latency;
+			conn_param.supervision_timeout =
+				chan->conn->hcon->hdev->le_supv_timeout;
+		}
+
+		hci_dev_unlock(hcon->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 &&
@@ -746,6 +788,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
 	struct bt_security sec;
 	struct bt_power pwr;
+	struct bt_le_conn_param conn_param;
 	struct l2cap_conn *conn;
 	int len, err = 0;
 	u32 opt;
@@ -761,6 +804,61 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 	lock_sock(sk);
 
 	switch (optname) {
+	case BT_LE_CONN_PARAM: {
+		struct hci_conn_params *hci_param;
+		struct hci_conn *hcon;
+
+		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;
+		}
+
+		if (hci_check_conn_params(conn_param.min_interval,
+			conn_param.max_interval, conn_param.latency,
+			conn_param.supervision_timeout) < 0) {
+			err = -EINVAL;
+			BT_ERR("Ignoring invalid connection parameters");
+			break;
+		}
+
+		hcon = chan->conn->hcon;
+
+		hci_dev_lock(hcon->hdev);
+
+		hci_conn_params_clear_disabled(hcon->hdev);
+
+		hci_param = hci_conn_params_add(hcon->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_param->conn_min_interval = conn_param.min_interval;
+		hci_param->conn_max_interval = conn_param.max_interval;
+		hci_param->conn_latency = conn_param.latency;
+		hci_param->supervision_timeout = conn_param.supervision_timeout;
+
+		hci_dev_unlock(hcon->hdev);
+
+		hci_le_conn_update(hcon, conn_param.min_interval,
+			conn_param.max_interval, conn_param.latency,
+			conn_param.supervision_timeout);
+		mgmt_new_conn_param(hcon->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 &&
-- 
2.11.0

Re: [RFC][PATCH BlueZ] Bluetooth: L2CAP: Add BT_LE_CONN_PARAM socket option

[Felipe Ferreri Tonello]

[-- Attachment #1: Type: text/plain, Size: 5606 bytes --]

CC'ed Marcel and Luiz for comments.

I have implemented and tested the functionality in the shared/att.[ch]
lib too, but I am waiting for your comments on the approach first.

On 03/02/17 21:36, Felipe F. Tonello 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.
> 
> 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.
> 
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
> ---
>  include/net/bluetooth/bluetooth.h |  8 ++++
>  net/bluetooth/l2cap_sock.c        | 98 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 106 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 a8ba752732c9..9c492730b081 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -498,6 +498,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
>  	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
>  	struct bt_security sec;
>  	struct bt_power pwr;
> +	struct bt_le_conn_param conn_param;
>  	int len, err = 0;
>  
>  	BT_DBG("sk %p", sk);
> @@ -514,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 hci_conn_params *params;
> +		struct hci_conn *hcon;
> +
> +		if (!chan->conn) {
> +			err = -ENOTCONN;
> +			break;
> +		}
> +
> +		hcon = chan->conn->hcon;
> +		hci_dev_lock(hcon->hdev);
> +
> +		params = hci_conn_params_lookup(hcon->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 =
> +				chan->conn->hcon->hdev->le_conn_min_interval;
> +			conn_param.max_interval =
> +				chan->conn->hcon->hdev->le_conn_max_interval;
> +			conn_param.latency =
> +				chan->conn->hcon->hdev->le_conn_latency;
> +			conn_param.supervision_timeout =
> +				chan->conn->hcon->hdev->le_supv_timeout;
> +		}
> +
> +		hci_dev_unlock(hcon->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 &&
> @@ -746,6 +788,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>  	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
>  	struct bt_security sec;
>  	struct bt_power pwr;
> +	struct bt_le_conn_param conn_param;
>  	struct l2cap_conn *conn;
>  	int len, err = 0;
>  	u32 opt;
> @@ -761,6 +804,61 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>  	lock_sock(sk);
>  
>  	switch (optname) {
> +	case BT_LE_CONN_PARAM: {
> +		struct hci_conn_params *hci_param;
> +		struct hci_conn *hcon;
> +
> +		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;
> +		}
> +
> +		if (hci_check_conn_params(conn_param.min_interval,
> +			conn_param.max_interval, conn_param.latency,
> +			conn_param.supervision_timeout) < 0) {
> +			err = -EINVAL;
> +			BT_ERR("Ignoring invalid connection parameters");
> +			break;
> +		}
> +
> +		hcon = chan->conn->hcon;
> +
> +		hci_dev_lock(hcon->hdev);
> +
> +		hci_conn_params_clear_disabled(hcon->hdev);
> +
> +		hci_param = hci_conn_params_add(hcon->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_param->conn_min_interval = conn_param.min_interval;
> +		hci_param->conn_max_interval = conn_param.max_interval;
> +		hci_param->conn_latency = conn_param.latency;
> +		hci_param->supervision_timeout = conn_param.supervision_timeout;
> +
> +		hci_dev_unlock(hcon->hdev);
> +
> +		hci_le_conn_update(hcon, conn_param.min_interval,
> +			conn_param.max_interval, conn_param.latency,
> +			conn_param.supervision_timeout);
> +		mgmt_new_conn_param(hcon->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 &&
> 

-- 
Felipe

[-- Attachment #2: 0x92698E6A.asc --]
[-- Type: application/pgp-keys, Size: 7177 bytes --]

Re: [RFC][PATCH BlueZ] Bluetooth: L2CAP: Add BT_LE_CONN_PARAM socket option

[Luiz Augusto von Dentz]

Hi Felipe,

On Wed, Feb 8, 2017 at 1:04 AM, Felipe Ferreri Tonello
<eu@felipetonello.com> wrote:
> CC'ed Marcel and Luiz for comments.
>
> I have implemented and tested the functionality in the shared/att.[ch]
> lib too, but I am waiting for your comments on the approach first.
>
> On 03/02/17 21:36, Felipe F. Tonello 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.
>>
>> 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.
>>
>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>> ---
>>  include/net/bluetooth/bluetooth.h |  8 ++++
>>  net/bluetooth/l2cap_sock.c        | 98 +++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 106 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 a8ba752732c9..9c492730b081 100644
>> --- a/net/bluetooth/l2cap_sock.c
>> +++ b/net/bluetooth/l2cap_sock.c
>> @@ -498,6 +498,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
>>       struct l2cap_chan *chan = l2cap_pi(sk)->chan;
>>       struct bt_security sec;
>>       struct bt_power pwr;
>> +     struct bt_le_conn_param conn_param;
>>       int len, err = 0;
>>
>>       BT_DBG("sk %p", sk);
>> @@ -514,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 hci_conn_params *params;
>> +             struct hci_conn *hcon;
>> +
>> +             if (!chan->conn) {
>> +                     err = -ENOTCONN;
>> +                     break;
>> +             }
>> +
>> +             hcon = chan->conn->hcon;
>> +             hci_dev_lock(hcon->hdev);
>> +
>> +             params = hci_conn_params_lookup(hcon->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 =
>> +                             chan->conn->hcon->hdev->le_conn_min_interval;
>> +                     conn_param.max_interval =
>> +                             chan->conn->hcon->hdev->le_conn_max_interval;
>> +                     conn_param.latency =
>> +                             chan->conn->hcon->hdev->le_conn_latency;
>> +                     conn_param.supervision_timeout =
>> +                             chan->conn->hcon->hdev->le_supv_timeout;
>> +             }
>> +
>> +             hci_dev_unlock(hcon->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 &&
>> @@ -746,6 +788,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>>       struct l2cap_chan *chan = l2cap_pi(sk)->chan;
>>       struct bt_security sec;
>>       struct bt_power pwr;
>> +     struct bt_le_conn_param conn_param;
>>       struct l2cap_conn *conn;
>>       int len, err = 0;
>>       u32 opt;
>> @@ -761,6 +804,61 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>>       lock_sock(sk);
>>
>>       switch (optname) {
>> +     case BT_LE_CONN_PARAM: {
>> +             struct hci_conn_params *hci_param;
>> +             struct hci_conn *hcon;
>> +
>> +             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;
>> +             }
>> +
>> +             if (hci_check_conn_params(conn_param.min_interval,
>> +                     conn_param.max_interval, conn_param.latency,
>> +                     conn_param.supervision_timeout) < 0) {
>> +                     err = -EINVAL;
>> +                     BT_ERR("Ignoring invalid connection parameters");
>> +                     break;
>> +             }
>> +
>> +             hcon = chan->conn->hcon;
>> +
>> +             hci_dev_lock(hcon->hdev);
>> +
>> +             hci_conn_params_clear_disabled(hcon->hdev);
>> +
>> +             hci_param = hci_conn_params_add(hcon->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_param->conn_min_interval = conn_param.min_interval;
>> +             hci_param->conn_max_interval = conn_param.max_interval;
>> +             hci_param->conn_latency = conn_param.latency;
>> +             hci_param->supervision_timeout = conn_param.supervision_timeout;
>> +
>> +             hci_dev_unlock(hcon->hdev);
>> +
>> +             hci_le_conn_update(hcon, conn_param.min_interval,
>> +                     conn_param.max_interval, conn_param.latency,
>> +                     conn_param.supervision_timeout);

Does hci_le_conn_update have any logic to mix different intervals,
from the looks of this it seems the last request would always win
instead what we probably want is to have the most aggressive
intervals, latency and timeout (or actually perhaps the most relaxed
timeout). Btw, in Zephyr we are doing something like this:

/*
* If remote does not support LL Connection Parameters Request
* Procedure
*/
if ((conn->role == BT_HCI_ROLE_SLAVE) &&
   !BT_FEAT_LE_CONN_PARAM_REQ_PROC(conn->le.features)) {
return bt_l2cap_update_conn_param(conn, param);

We should have something similar here.

>> +             mgmt_new_conn_param(hcon->hdev,
>> +                     &hcon->dst, hcon->dst_type,
>> +                     true, conn_param.min_interval, conn_param.max_interval,
>> +                     conn_param.latency, conn_param.supervision_timeout);

iirc the parameter can be rejected, also is this a blocking API? Im
afraid if we do end up having to update the parameters using L2CAP
procedure the MGMT command may only be sent once that finishes, in
that case it has to be asynchronous at least from the kernel point of
view.

>> +             break;
>> +     }
>> +
>>       case BT_SECURITY:
>>               if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED &&
>>                   chan->chan_type != L2CAP_CHAN_FIXED &&
>>
>
> --
> Felipe



-- 
Luiz Augusto von Dentz

Re: [RFC][PATCH BlueZ] Bluetooth: L2CAP: Add BT_LE_CONN_PARAM socket option

[Felipe Ferreri Tonello]

[-- Attachment #1: Type: text/plain, Size: 9466 bytes --]

Hi Luiz,

On 08/02/17 09:34, Luiz Augusto von Dentz wrote:
> Hi Felipe,
> 
> On Wed, Feb 8, 2017 at 1:04 AM, Felipe Ferreri Tonello
> <eu@felipetonello.com> wrote:
>> CC'ed Marcel and Luiz for comments.
>>
>> I have implemented and tested the functionality in the shared/att.[ch]
>> lib too, but I am waiting for your comments on the approach first.
>>
>> On 03/02/17 21:36, Felipe F. Tonello 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.
>>>
>>> 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.
>>>
>>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>>> ---
>>>  include/net/bluetooth/bluetooth.h |  8 ++++
>>>  net/bluetooth/l2cap_sock.c        | 98 +++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 106 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 a8ba752732c9..9c492730b081 100644
>>> --- a/net/bluetooth/l2cap_sock.c
>>> +++ b/net/bluetooth/l2cap_sock.c
>>> @@ -498,6 +498,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
>>>       struct l2cap_chan *chan = l2cap_pi(sk)->chan;
>>>       struct bt_security sec;
>>>       struct bt_power pwr;
>>> +     struct bt_le_conn_param conn_param;
>>>       int len, err = 0;
>>>
>>>       BT_DBG("sk %p", sk);
>>> @@ -514,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 hci_conn_params *params;
>>> +             struct hci_conn *hcon;
>>> +
>>> +             if (!chan->conn) {
>>> +                     err = -ENOTCONN;
>>> +                     break;
>>> +             }
>>> +
>>> +             hcon = chan->conn->hcon;
>>> +             hci_dev_lock(hcon->hdev);
>>> +
>>> +             params = hci_conn_params_lookup(hcon->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 =
>>> +                             chan->conn->hcon->hdev->le_conn_min_interval;
>>> +                     conn_param.max_interval =
>>> +                             chan->conn->hcon->hdev->le_conn_max_interval;
>>> +                     conn_param.latency =
>>> +                             chan->conn->hcon->hdev->le_conn_latency;
>>> +                     conn_param.supervision_timeout =
>>> +                             chan->conn->hcon->hdev->le_supv_timeout;
>>> +             }
>>> +
>>> +             hci_dev_unlock(hcon->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 &&
>>> @@ -746,6 +788,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>>>       struct l2cap_chan *chan = l2cap_pi(sk)->chan;
>>>       struct bt_security sec;
>>>       struct bt_power pwr;
>>> +     struct bt_le_conn_param conn_param;
>>>       struct l2cap_conn *conn;
>>>       int len, err = 0;
>>>       u32 opt;
>>> @@ -761,6 +804,61 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>>>       lock_sock(sk);
>>>
>>>       switch (optname) {
>>> +     case BT_LE_CONN_PARAM: {
>>> +             struct hci_conn_params *hci_param;
>>> +             struct hci_conn *hcon;
>>> +
>>> +             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;
>>> +             }
>>> +
>>> +             if (hci_check_conn_params(conn_param.min_interval,
>>> +                     conn_param.max_interval, conn_param.latency,
>>> +                     conn_param.supervision_timeout) < 0) {
>>> +                     err = -EINVAL;
>>> +                     BT_ERR("Ignoring invalid connection parameters");
>>> +                     break;
>>> +             }
>>> +
>>> +             hcon = chan->conn->hcon;
>>> +
>>> +             hci_dev_lock(hcon->hdev);
>>> +
>>> +             hci_conn_params_clear_disabled(hcon->hdev);
>>> +
>>> +             hci_param = hci_conn_params_add(hcon->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_param->conn_min_interval = conn_param.min_interval;
>>> +             hci_param->conn_max_interval = conn_param.max_interval;
>>> +             hci_param->conn_latency = conn_param.latency;
>>> +             hci_param->supervision_timeout = conn_param.supervision_timeout;
>>> +
>>> +             hci_dev_unlock(hcon->hdev);
>>> +
>>> +             hci_le_conn_update(hcon, conn_param.min_interval,
>>> +                     conn_param.max_interval, conn_param.latency,
>>> +                     conn_param.supervision_timeout);
> 
> Does hci_le_conn_update have any logic to mix different intervals,
> from the looks of this it seems the last request would always win
> instead what we probably want is to have the most aggressive
> intervals, latency and timeout (or actually perhaps the most relaxed
> timeout). Btw, in Zephyr we are doing something like this:

No, this simply sends a HCI command to the controller and saves the
parameters in cache.

> 
> /*
> * If remote does not support LL Connection Parameters Request
> * Procedure
> */
> if ((conn->role == BT_HCI_ROLE_SLAVE) &&
>    !BT_FEAT_LE_CONN_PARAM_REQ_PROC(conn->le.features)) {
> return bt_l2cap_update_conn_param(conn, param);
> 
> We should have something similar here.

In this case, if it is HCI_ROLE_MASTER it will not be able to update?

What is BT_FEAT_LE_CONN_PARAM_REQ_PROC() doing?

Ideally we should use the this procedure if master and slave supports
4.1 or greater. So they both are in sync (with the same conn_param) right?

What is the proper way of doing on a 4.0 then? AFAIK this command is
supposed to be supported only by the Master. (More below)

> 
>>> +             mgmt_new_conn_param(hcon->hdev,
>>> +                     &hcon->dst, hcon->dst_type,
>>> +                     true, conn_param.min_interval, conn_param.max_interval,
>>> +                     conn_param.latency, conn_param.supervision_timeout);
> 
> iirc the parameter can be rejected, also is this a blocking API? Im
> afraid if we do end up having to update the parameters using L2CAP
> procedure the MGMT command may only be sent once that finishes, in
> that case it has to be asynchronous at least from the kernel point of
> view.

The BT 5.0 says this about the LE Connection Update Complete Event:

Note: The parameter values returned in this event may be different from
the parameter values provided by the Host through the LE Connection
Update command (Section 7.8.18)

But on 4.0 it says:
On a slave, if no connection parameters are updated, then this event
shall not be issued.
On a master, this event shall be issued if the Connection_Update command
was sent.

===

So yes, I agree that the appropriate thing to do is send the MGMT
command from the event handler, right?

Also, based on the 4.0 spec, how can we trigger the LE Connection Update
Complete Event as a Slave?


> 
>>> +             break;
>>> +     }
>>> +
>>>       case BT_SECURITY:
>>>               if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED &&
>>>                   chan->chan_type != L2CAP_CHAN_FIXED &&
>>>
>>
>> --
>> Felipe
> 
> 
> 

-- 
Felipe

[-- Attachment #2: 0x92698E6A.asc --]
[-- Type: application/pgp-keys, Size: 7291 bytes --]

Re: [RFC][PATCH BlueZ] Bluetooth: L2CAP: Add BT_LE_CONN_PARAM socket option

[Felipe Ferreri Tonello]

[-- Attachment #1: Type: text/plain, Size: 10083 bytes --]

Hi Luiz,

On 09/02/17 17:14, Felipe Ferreri Tonello wrote:
> Hi Luiz,
> 
> On 08/02/17 09:34, Luiz Augusto von Dentz wrote:
>> Hi Felipe,
>>
>> On Wed, Feb 8, 2017 at 1:04 AM, Felipe Ferreri Tonello
>> <eu@felipetonello.com> wrote:
>>> CC'ed Marcel and Luiz for comments.
>>>
>>> I have implemented and tested the functionality in the shared/att.[ch]
>>> lib too, but I am waiting for your comments on the approach first.
>>>
>>> On 03/02/17 21:36, Felipe F. Tonello 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.
>>>>
>>>> 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.
>>>>
>>>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>>>> ---
>>>>  include/net/bluetooth/bluetooth.h |  8 ++++
>>>>  net/bluetooth/l2cap_sock.c        | 98 +++++++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 106 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 a8ba752732c9..9c492730b081 100644
>>>> --- a/net/bluetooth/l2cap_sock.c
>>>> +++ b/net/bluetooth/l2cap_sock.c
>>>> @@ -498,6 +498,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
>>>>       struct l2cap_chan *chan = l2cap_pi(sk)->chan;
>>>>       struct bt_security sec;
>>>>       struct bt_power pwr;
>>>> +     struct bt_le_conn_param conn_param;
>>>>       int len, err = 0;
>>>>
>>>>       BT_DBG("sk %p", sk);
>>>> @@ -514,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 hci_conn_params *params;
>>>> +             struct hci_conn *hcon;
>>>> +
>>>> +             if (!chan->conn) {
>>>> +                     err = -ENOTCONN;
>>>> +                     break;
>>>> +             }
>>>> +
>>>> +             hcon = chan->conn->hcon;
>>>> +             hci_dev_lock(hcon->hdev);
>>>> +
>>>> +             params = hci_conn_params_lookup(hcon->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 =
>>>> +                             chan->conn->hcon->hdev->le_conn_min_interval;
>>>> +                     conn_param.max_interval =
>>>> +                             chan->conn->hcon->hdev->le_conn_max_interval;
>>>> +                     conn_param.latency =
>>>> +                             chan->conn->hcon->hdev->le_conn_latency;
>>>> +                     conn_param.supervision_timeout =
>>>> +                             chan->conn->hcon->hdev->le_supv_timeout;
>>>> +             }
>>>> +
>>>> +             hci_dev_unlock(hcon->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 &&
>>>> @@ -746,6 +788,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>>>>       struct l2cap_chan *chan = l2cap_pi(sk)->chan;
>>>>       struct bt_security sec;
>>>>       struct bt_power pwr;
>>>> +     struct bt_le_conn_param conn_param;
>>>>       struct l2cap_conn *conn;
>>>>       int len, err = 0;
>>>>       u32 opt;
>>>> @@ -761,6 +804,61 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>>>>       lock_sock(sk);
>>>>
>>>>       switch (optname) {
>>>> +     case BT_LE_CONN_PARAM: {
>>>> +             struct hci_conn_params *hci_param;
>>>> +             struct hci_conn *hcon;
>>>> +
>>>> +             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;
>>>> +             }
>>>> +
>>>> +             if (hci_check_conn_params(conn_param.min_interval,
>>>> +                     conn_param.max_interval, conn_param.latency,
>>>> +                     conn_param.supervision_timeout) < 0) {
>>>> +                     err = -EINVAL;
>>>> +                     BT_ERR("Ignoring invalid connection parameters");
>>>> +                     break;
>>>> +             }
>>>> +
>>>> +             hcon = chan->conn->hcon;
>>>> +
>>>> +             hci_dev_lock(hcon->hdev);
>>>> +
>>>> +             hci_conn_params_clear_disabled(hcon->hdev);
>>>> +
>>>> +             hci_param = hci_conn_params_add(hcon->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_param->conn_min_interval = conn_param.min_interval;
>>>> +             hci_param->conn_max_interval = conn_param.max_interval;
>>>> +             hci_param->conn_latency = conn_param.latency;
>>>> +             hci_param->supervision_timeout = conn_param.supervision_timeout;
>>>> +
>>>> +             hci_dev_unlock(hcon->hdev);
>>>> +
>>>> +             hci_le_conn_update(hcon, conn_param.min_interval,
>>>> +                     conn_param.max_interval, conn_param.latency,
>>>> +                     conn_param.supervision_timeout);
>>
>> Does hci_le_conn_update have any logic to mix different intervals,
>> from the looks of this it seems the last request would always win
>> instead what we probably want is to have the most aggressive
>> intervals, latency and timeout (or actually perhaps the most relaxed
>> timeout). Btw, in Zephyr we are doing something like this:
> 
> No, this simply sends a HCI command to the controller and saves the
> parameters in cache.
> 
>>
>> /*
>> * If remote does not support LL Connection Parameters Request
>> * Procedure
>> */
>> if ((conn->role == BT_HCI_ROLE_SLAVE) &&
>>    !BT_FEAT_LE_CONN_PARAM_REQ_PROC(conn->le.features)) {
>> return bt_l2cap_update_conn_param(conn, param);
>>
>> We should have something similar here.
> 
> In this case, if it is HCI_ROLE_MASTER it will not be able to update?
> 
> What is BT_FEAT_LE_CONN_PARAM_REQ_PROC() doing?
> 
> Ideally we should use the this procedure if master and slave supports
> 4.1 or greater. So they both are in sync (with the same conn_param) right?
> 
> What is the proper way of doing on a 4.0 then? AFAIK this command is
> supposed to be supported only by the Master. (More below)
> 
>>
>>>> +             mgmt_new_conn_param(hcon->hdev,
>>>> +                     &hcon->dst, hcon->dst_type,
>>>> +                     true, conn_param.min_interval, conn_param.max_interval,
>>>> +                     conn_param.latency, conn_param.supervision_timeout);
>>
>> iirc the parameter can be rejected, also is this a blocking API? Im
>> afraid if we do end up having to update the parameters using L2CAP
>> procedure the MGMT command may only be sent once that finishes, in
>> that case it has to be asynchronous at least from the kernel point of
>> view.
> 
> The BT 5.0 says this about the LE Connection Update Complete Event:
> 
> Note: The parameter values returned in this event may be different from
> the parameter values provided by the Host through the LE Connection
> Update command (Section 7.8.18)
> 
> But on 4.0 it says:
> On a slave, if no connection parameters are updated, then this event
> shall not be issued.
> On a master, this event shall be issued if the Connection_Update command
> was sent.
> 
> ===
> 
> So yes, I agree that the appropriate thing to do is send the MGMT
> command from the event handler, right?

I've been looking into this right now, and I believe we should send the
MGMT command in the sockopt instead (how this patch is doing already).

The reason is that bluetoothd (or the MGMT command itself) stores the
data used on the HCI Connection Update command and not the actual value
of the connection.

> 
> Also, based on the 4.0 spec, how can we trigger the LE Connection Update
> Complete Event as a Slave?
> 
> 
>>
>>>> +             break;
>>>> +     }
>>>> +
>>>>       case BT_SECURITY:
>>>>               if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED &&
>>>>                   chan->chan_type != L2CAP_CHAN_FIXED &&
>>>>
>>
> 

-- 
Felipe

[-- Attachment #2: 0x92698E6A.asc --]
[-- Type: application/pgp-keys, Size: 7177 bytes --]

Re: [RFC][PATCH BlueZ] Bluetooth: L2CAP: Add BT_LE_CONN_PARAM socket option

[Marcel Holtmann]

Hi Felipe,

> 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.
> 
> 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.
> 
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
> ---
> include/net/bluetooth/bluetooth.h |  8 ++++
> net/bluetooth/l2cap_sock.c        | 98 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 106 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;
> +};
> +

I am not 100% convinced that we want this level of detail exposed to user space. I was more thinking of having two or three sets of connection parameters and then a simple enum via socket option to choose from.

We have the default value that we use for everything right now. And then we can have a high latency and low latency setting to change that. The supervision timeout is a different story since that would also apply to BR/EDR and we could allow for individual setting of that.

The reason we store the exact connection parameters via mgmt is so that next time around, the slave does not have to request new ones. We already use the correct ones from the get go. One thing we are missing is to actually pick them up from advertising report. Since they can also be reported that way.

Also with a socket option we have to be careful that multiple sockets might want to program different values. We do support connection oriented channels with LE. Otherwise this would have to artificially restricted to GATT CID for now. And even there is still the case between connect() and accept(). Also that accept() is use with the background connection. I mean on an outgoing connection, I want to be able to influence the settings before calling connect() so they are used directly in the LE_Create_Connection command. Especially if I already know what I am creating the connection for.

Regards

Marcel

Re: [RFC][PATCH BlueZ] Bluetooth: L2CAP: Add BT_LE_CONN_PARAM socket option

[Felipe Ferreri Tonello]

[-- Attachment #1: Type: text/plain, Size: 3888 bytes --]

Hi Marcel,

On 10/02/17 11:30, Marcel Holtmann wrote:
> Hi Felipe,
> 
>> 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.
>>
>> 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.
>>
>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>> ---
>> include/net/bluetooth/bluetooth.h |  8 ++++
>> net/bluetooth/l2cap_sock.c        | 98 +++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 106 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;
>> +};
>> +
> 
> I am not 100% convinced that we want this level of detail exposed to user space. I was more thinking of having two or three sets of connection parameters and then a simple enum via socket option to choose from.
> 
> We have the default value that we use for everything right now. And then we can have a high latency and low latency setting to change that. The supervision timeout is a different story since that would also apply to BR/EDR and we could allow for individual setting of that.

Ok, so we could have:
 - LOW_LATENCY: minimum possible values
 - DEFAULT_LATENCY: default
 - HIGH_LATENCY: Maximum possible values

When you say latency, do you only refer to the latency field or the
min/max_interval too?

> 
> The reason we store the exact connection parameters via mgmt is so that next time around, the slave does not have to request new ones. We already use the correct ones from the get go. One thing we are missing is to actually pick them up from advertising report. Since they can also be reported that way.

Ok, but what about the Master? Are you talking about the
LL_CONNECTION_PARAM_REQ procedure from 4.1? If so, it should work both
ways, right?

> 
> Also with a socket option we have to be careful that multiple sockets might want to program different values.

Right. That can be handled in bluetoothd, probably.

> We do support connection oriented channels with LE. Otherwise this would have to artificially restricted to GATT CID for now.

How would this affect? Sorry, I am not familiar with CoC.

> And even there is still the case between connect() and accept(). Also that accept() is use with the background connection. 

What is the case between connect() and accept()?

> I mean on an outgoing connection, I want to be able to influence the settings before calling connect() so they are used directly in the LE_Create_Connection command. Especially if I already know what I am creating the connection for.

Yes, in the case the connection parameter is in the advertising data, we
can definitely do it. Perhaps saving in the device info file before the
device_create() in bluetoothd is called.

But like I said, there is also the case where the slave doesn't provide
this information and the connection needs to be updated anyway based on
the profile (which is my use-case after all).

====

TBH I am very confused with all the possible ways slave and master can
update its connection parameters and vice-versa. It also has differences
between 4.0 and 4.1 and greater.

Could we list these first and so we have a clear picture on what it
needs to be done?

Thanks

-- 
Felipe

[-- Attachment #2: 0x92698E6A.asc --]
[-- Type: application/pgp-keys, Size: 7291 bytes --]