[RFC v2 1/4] Bluetooth: Connection parameters check helper

[RFC v2 1/4] Bluetooth: Connection parameters check helper

[Andre Guedes]

This patch renames l2cap_check_conn_param() to hci_check_conn_params()
and moves it to hci_core.h so it can reused in others files. This helper
will be reused in the next patch.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h | 21 +++++++++++++++++++++
 net/bluetooth/l2cap_core.c       | 23 +----------------------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index dda7f00..016d262 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1173,6 +1173,27 @@ static inline struct smp_irk *hci_get_irk(struct hci_dev *hdev,
 	return hci_find_irk_by_rpa(hdev, bdaddr);
 }
 
+static inline int hci_check_conn_params(u16 min, u16 max, u16 latency,
+					u16 to_multiplier)
+{
+	u16 max_latency;
+
+	if (min > max || min < 6 || max > 3200)
+		return -EINVAL;
+
+	if (to_multiplier < 10 || to_multiplier > 3200)
+		return -EINVAL;
+
+	if (max >= to_multiplier * 8)
+		return -EINVAL;
+
+	max_latency = (to_multiplier * 8 / max) - 1;
+	if (latency > 499 || latency > max_latency)
+		return -EINVAL;
+
+	return 0;
+}
+
 int hci_register_cb(struct hci_cb *hcb);
 int hci_unregister_cb(struct hci_cb *hcb);
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d015aa1..e203a5f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5213,27 +5213,6 @@ static inline int l2cap_move_channel_confirm_rsp(struct l2cap_conn *conn,
 	return 0;
 }
 
-static inline int l2cap_check_conn_param(u16 min, u16 max, u16 latency,
-					 u16 to_multiplier)
-{
-	u16 max_latency;
-
-	if (min > max || min < 6 || max > 3200)
-		return -EINVAL;
-
-	if (to_multiplier < 10 || to_multiplier > 3200)
-		return -EINVAL;
-
-	if (max >= to_multiplier * 8)
-		return -EINVAL;
-
-	max_latency = (to_multiplier * 8 / max) - 1;
-	if (latency > 499 || latency > max_latency)
-		return -EINVAL;
-
-	return 0;
-}
-
 static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
 					      struct l2cap_cmd_hdr *cmd,
 					      u16 cmd_len, u8 *data)
@@ -5261,7 +5240,7 @@ static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
 
 	memset(&rsp, 0, sizeof(rsp));
 
-	err = l2cap_check_conn_param(min, max, latency, to_multiplier);
+	err = hci_check_conn_params(min, max, latency, to_multiplier);
 	if (err)
 		rsp.result = cpu_to_le16(L2CAP_CONN_PARAM_REJECTED);
 	else
-- 
1.9.1

[RFC v2 2/4] Bluetooth: Connection Parameter Update Procedure

[Andre Guedes]

This patch adds support for LE Connection Parameters Request Link
Layer control procedure introduced in Core spec 4.1. This procedure
allows a Peripheral or Central to update the Link Layer connection
parameters of an established connection.

Regarding the acceptance of connection parameters, the LL procedure
follows the same approach of L2CAP procedure (see l2cap_conn_param_
update_req function). We accept any connection parameters values as
long as they are within the valid range.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci.h | 28 +++++++++++++++++++++++++
 net/bluetooth/hci_event.c   | 50 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index cc2e88d..59bad0b 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -355,6 +355,7 @@ enum {
 #define HCI_LK_AUTH_COMBINATION_P256	0x08
 
 /* ---- HCI Error Codes ---- */
+#define HCI_ERROR_UNKNOWN_CONN_ID	0x02
 #define HCI_ERROR_AUTH_FAILURE		0x05
 #define HCI_ERROR_MEMORY_EXCEEDED	0x07
 #define HCI_ERROR_CONNECTION_TIMEOUT	0x08
@@ -364,6 +365,7 @@ enum {
 #define HCI_ERROR_REMOTE_POWER_OFF	0x15
 #define HCI_ERROR_LOCAL_HOST_TERM	0x16
 #define HCI_ERROR_PAIRING_NOT_ALLOWED	0x18
+#define HCI_ERROR_INVALID_LL_PARAMS	0x1E
 #define HCI_ERROR_ADVERTISING_TIMEOUT	0x3c
 
 /* Flow control modes */
@@ -1288,6 +1290,23 @@ struct hci_rp_le_read_supported_states {
 	__u8	le_states[8];
 } __packed;
 
+#define HCI_OP_LE_CONN_PARAM_REQ_REPLY	0x2020
+struct hci_cp_le_conn_param_req_reply {
+	__le16	handle;
+	__le16	interval_min;
+	__le16	interval_max;
+	__le16	latency;
+	__le16	timeout;
+	__le16	min_ce_len;
+	__le16	max_ce_len;
+} __packed;
+
+#define HCI_OP_LE_CONN_PARAM_REQ_NEG_REPLY	0x2021
+struct hci_cp_le_conn_param_req_neg_reply {
+	__le16	handle;
+	__u8	reason;
+} __packed;
+
 /* ---- HCI Events ---- */
 #define HCI_EV_INQUIRY_COMPLETE		0x01
 
@@ -1683,6 +1702,15 @@ struct hci_ev_le_ltk_req {
 	__le16	ediv;
 } __packed;
 
+#define HCI_EV_LE_REMOTE_CONN_PARAM_REQ	0x06
+struct hci_ev_le_remote_conn_param_req {
+	__le16 handle;
+	__le16 interval_min;
+	__le16 interval_max;
+	__le16 latency;
+	__le16 timeout;
+} __packed;
+
 /* Advertising report event types */
 #define LE_ADV_IND		0x00
 #define LE_ADV_DIRECT_IND	0x01
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 7a23324..4ded97b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4309,6 +4309,52 @@ not_found:
 	hci_dev_unlock(hdev);
 }
 
+static void send_conn_param_neg_reply(struct hci_dev *hdev, u16 handle,
+				      u8 reason)
+{
+	struct hci_cp_le_conn_param_req_neg_reply cp;
+
+	cp.handle = cpu_to_le16(handle);
+	cp.reason = reason;
+
+	hci_send_cmd(hdev, HCI_OP_LE_CONN_PARAM_REQ_NEG_REPLY, sizeof(cp),
+		     &cp);
+}
+
+static void hci_le_remote_conn_param_req_evt(struct hci_dev *hdev,
+					     struct sk_buff *skb)
+{
+	struct hci_ev_le_remote_conn_param_req *ev = (void *) skb->data;
+	struct hci_cp_le_conn_param_req_reply cp;
+	struct hci_conn *hcon;
+	u16 handle, min, max, latency, timeout;
+
+	handle = le16_to_cpu(ev->handle);
+	min = le16_to_cpu(ev->interval_min);
+	max = le16_to_cpu(ev->interval_max);
+	latency = le16_to_cpu(ev->latency);
+	timeout = le16_to_cpu(ev->timeout);
+
+	hcon = hci_conn_hash_lookup_handle(hdev, handle);
+	if (!hcon || hcon->state != BT_CONNECTED)
+		return send_conn_param_neg_reply(hdev, handle,
+						 HCI_ERROR_UNKNOWN_CONN_ID);
+
+	if (hci_check_conn_params(min, max, latency, timeout))
+		return send_conn_param_neg_reply(hdev, handle,
+						 HCI_ERROR_INVALID_LL_PARAMS);
+
+	cp.handle = ev->handle;
+	cp.interval_min = ev->interval_min;
+	cp.interval_max = ev->interval_max;
+	cp.latency = ev->latency;
+	cp.timeout = ev->timeout;
+	cp.min_ce_len = 0;
+	cp.max_ce_len = 0;
+
+	hci_send_cmd(hdev, HCI_OP_LE_CONN_PARAM_REQ_REPLY, sizeof(cp), &cp);
+}
+
 static void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_le_meta *le_ev = (void *) skb->data;
@@ -4332,6 +4378,10 @@ static void hci_le_meta_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_le_ltk_request_evt(hdev, skb);
 		break;
 
+	case HCI_EV_LE_REMOTE_CONN_PARAM_REQ:
+		hci_le_remote_conn_param_req_evt(hdev, skb);
+		break;
+
 	default:
 		break;
 	}
-- 
1.9.1

[RFC v2 3/4] Bluetooth: Enable new LE meta event

[Andre Guedes]

The Bluetooth 4.1 introduces a new LE meta event called "LE Remote
Connection Parameter Request" event. In order to the controller sends
this event to host, we should enable it during controller initialization.

Therefore, this patch enables that event in case the controller supports
Bluetooth 4.1 or upper version.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/bluetooth.h | 1 +
 net/bluetooth/hci_core.c          | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 373000d..85dbc03 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -38,6 +38,7 @@
 #define BLUETOOTH_VER_1_1	1
 #define BLUETOOTH_VER_1_2	2
 #define BLUETOOTH_VER_2_0	3
+#define BLUETOOTH_VER_4_1	7
 
 /* Reserv for core and drivers use */
 #define BT_SKB_RESERVE	8
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9852449..a8b9d70 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1461,6 +1461,13 @@ static void hci_setup_event_mask(struct hci_request *req)
 	if (lmp_le_capable(hdev)) {
 		memset(events, 0, sizeof(events));
 		events[0] = 0x1f;
+
+		/* If Bluetooth 4.1 or upper version, enable LE Remote
+		 * Connection Parameter Request Event.
+		 */
+		if (hdev->hci_ver >= BLUETOOTH_VER_4_1)
+			events[0] = events[0] | 0x20;
+
 		hci_req_add(req, HCI_OP_LE_SET_EVENT_MASK,
 			    sizeof(events), events);
 	}
-- 
1.9.1

[RFC v2 4/4] Bluetooth: Introduce "New Connection Parameter" Event

[Andre Guedes]

This patch introduces a new Mgmt event called "New Connection Parameter".
This event indicates to userspace the connection parameters values the
remote device requested.

The user may store these values and load them into kernel (an upcoming
patch will introduce a Mgmt command to do that). This way, next time a
connection is established to that device, the kernel will use those
parameters values instead of the default ones.

This event is sent when the remote device requests new connection
parameters through Link Layer or L2CAP connection parameter update
procedures.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |  3 +++
 include/net/bluetooth/mgmt.h     | 10 ++++++++++
 net/bluetooth/hci_event.c        |  3 +++
 net/bluetooth/l2cap_core.c       |  6 +++++-
 net/bluetooth/mgmt.c             | 19 +++++++++++++++++++
 5 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 016d262..f318efe 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1322,6 +1322,9 @@ void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent);
 void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk);
 void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
 		   bool persistent);
+void mgmt_new_conn_param(struct hci_dev *hdev, bdaddr_t *bdaddr,
+			 u8 bdaddr_type, u16 min_interval, u16 max_interval,
+			 u16 latency, u16 timeout);
 void mgmt_reenable_advertising(struct hci_dev *hdev);
 void mgmt_smp_complete(struct hci_conn *conn, bool complete);
 
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index bcffc9a..de22982 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -578,3 +578,13 @@ struct mgmt_ev_new_csrk {
 	__u8 store_hint;
 	struct mgmt_csrk_info key;
 } __packed;
+
+#define MGMT_EV_NEW_CONN_PARAM		0x001a
+struct mgmt_ev_new_conn_param {
+	struct mgmt_addr_info addr;
+	__u8 store_hint;
+	__le16 min_interval;
+	__le16 max_interval;
+	__le16 latency;
+	__le16 timeout;
+} __packed;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 4ded97b..0a103ad 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4344,6 +4344,9 @@ static void hci_le_remote_conn_param_req_evt(struct hci_dev *hdev,
 		return send_conn_param_neg_reply(hdev, handle,
 						 HCI_ERROR_INVALID_LL_PARAMS);
 
+	mgmt_new_conn_param(hdev, &hcon->dst, hcon->dst_type, min, max,
+			    latency, timeout);
+
 	cp.handle = ev->handle;
 	cp.interval_min = ev->interval_min;
 	cp.interval_max = ev->interval_max;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index e203a5f..058b3b2 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5249,8 +5249,12 @@ static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
 	l2cap_send_cmd(conn, cmd->ident, L2CAP_CONN_PARAM_UPDATE_RSP,
 		       sizeof(rsp), &rsp);
 
-	if (!err)
+	if (!err) {
+		mgmt_new_conn_param(hcon->hdev, &hcon->dst, hcon->dst_type,
+				    min, max, latency, to_multiplier);
+
 		hci_le_conn_update(hcon, min, max, latency, to_multiplier);
+	}
 
 	return 0;
 }
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index f75a090..ca96c4c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -111,6 +111,7 @@ static const u16 mgmt_events[] = {
 	MGMT_EV_PASSKEY_NOTIFY,
 	MGMT_EV_NEW_IRK,
 	MGMT_EV_NEW_CSRK,
+	MGMT_EV_NEW_CONN_PARAM,
 };
 
 #define CACHE_TIMEOUT	msecs_to_jiffies(2 * 1000)
@@ -5384,6 +5385,24 @@ void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
 	mgmt_event(MGMT_EV_NEW_CSRK, hdev, &ev, sizeof(ev), NULL);
 }
 
+void mgmt_new_conn_param(struct hci_dev *hdev, bdaddr_t *bdaddr,
+			 u8 bdaddr_type, u16 min_interval, u16 max_interval,
+			 u16 latency, u16 timeout)
+{
+	struct mgmt_ev_new_conn_param ev;
+
+	memset(&ev, 0, sizeof(ev));
+	bacpy(&ev.addr.bdaddr, bdaddr);
+	ev.addr.type = link_to_bdaddr(LE_LINK, bdaddr_type);
+	ev.store_hint = 0x00;
+	ev.min_interval = cpu_to_le16(min_interval);
+	ev.max_interval = cpu_to_le16(max_interval);
+	ev.latency = cpu_to_le16(latency);
+	ev.timeout = cpu_to_le16(timeout);
+
+	mgmt_event(MGMT_EV_NEW_CONN_PARAM, hdev, &ev, sizeof(ev), NULL);
+}
+
 static inline u16 eir_append_data(u8 *eir, u16 eir_len, u8 type, u8 *data,
 				  u8 data_len)
 {
-- 
1.9.1

Re: [RFC v2 3/4] Bluetooth: Enable new LE meta event

[Marcel Holtmann]

Hi Andre,

> The Bluetooth 4.1 introduces a new LE meta event called "LE Remote
> Connection Parameter Request" event. In order to the controller sends
> this event to host, we should enable it during controller initialization.
> 
> Therefore, this patch enables that event in case the controller supports
> Bluetooth 4.1 or upper version.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/bluetooth.h | 1 +
> net/bluetooth/hci_core.c          | 7 +++++++
> 2 files changed, 8 insertions(+)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 373000d..85dbc03 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -38,6 +38,7 @@
> #define BLUETOOTH_VER_1_1	1
> #define BLUETOOTH_VER_1_2	2
> #define BLUETOOTH_VER_2_0	3
> +#define BLUETOOTH_VER_4_1	7
> 
> /* Reserv for core and drivers use */
> #define BT_SKB_RESERVE	8
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9852449..a8b9d70 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1461,6 +1461,13 @@ static void hci_setup_event_mask(struct hci_request *req)
> 	if (lmp_le_capable(hdev)) {
> 		memset(events, 0, sizeof(events));
> 		events[0] = 0x1f;
> +
> +		/* If Bluetooth 4.1 or upper version, enable LE Remote
> +		 * Connection Parameter Request Event.
> +		 */
> +		if (hdev->hci_ver >= BLUETOOTH_VER_4_1)
> +			events[0] = events[0] | 0x20;
> +

we only fall back to Bluetooth version check in case we can not feature masks to determine if something is supported or not. Use the LE features mask to figure out if we support this or not.

Regards

Marcel

Re: [RFC v2 4/4] Bluetooth: Introduce "New Connection Parameter" Event

[Marcel Holtmann]

Hi Andre,

> This patch introduces a new Mgmt event called "New Connection Parameter".
> This event indicates to userspace the connection parameters values the
> remote device requested.
> 
> The user may store these values and load them into kernel (an upcoming
> patch will introduce a Mgmt command to do that). This way, next time a
> connection is established to that device, the kernel will use those
> parameters values instead of the default ones.
> 
> This event is sent when the remote device requests new connection
> parameters through Link Layer or L2CAP connection parameter update
> procedures.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h |  3 +++
> include/net/bluetooth/mgmt.h     | 10 ++++++++++
> net/bluetooth/hci_event.c        |  3 +++
> net/bluetooth/l2cap_core.c       |  6 +++++-
> net/bluetooth/mgmt.c             | 19 +++++++++++++++++++
> 5 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 016d262..f318efe 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1322,6 +1322,9 @@ void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent);
> void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk);
> void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
> 		   bool persistent);
> +void mgmt_new_conn_param(struct hci_dev *hdev, bdaddr_t *bdaddr,
> +			 u8 bdaddr_type, u16 min_interval, u16 max_interval,
> +			 u16 latency, u16 timeout);
> void mgmt_reenable_advertising(struct hci_dev *hdev);
> void mgmt_smp_complete(struct hci_conn *conn, bool complete);
> 
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index bcffc9a..de22982 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -578,3 +578,13 @@ struct mgmt_ev_new_csrk {
> 	__u8 store_hint;
> 	struct mgmt_csrk_info key;
> } __packed;
> +
> +#define MGMT_EV_NEW_CONN_PARAM		0x001a
> +struct mgmt_ev_new_conn_param {
> +	struct mgmt_addr_info addr;
> +	__u8 store_hint;
> +	__le16 min_interval;
> +	__le16 max_interval;
> +	__le16 latency;
> +	__le16 timeout;
> +} __packed;

the only open question I still have is if we want to add a __u8 master variable here. The connection parameters are no longer strictly speaking slave to master only. With link layer topology, there are a bit more flexible.

In addition it could be possible that we also act as slave and in that case we request the parameters. I wonder if that should be done via this event as well or if we just use socket option for that. Thoughts?

Regards

Marcel

Re: [RFC v2 1/4] Bluetooth: Connection parameters check helper

[Marcel Holtmann]

Hi Andre,

> This patch renames l2cap_check_conn_param() to hci_check_conn_params()
> and moves it to hci_core.h so it can reused in others files. This helper
> will be reused in the next patch.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h | 21 +++++++++++++++++++++
> net/bluetooth/l2cap_core.c       | 23 +----------------------
> 2 files changed, 22 insertions(+), 22 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [RFC v2 3/4] Bluetooth: Enable new LE meta event

[Andre Guedes]

Hi Marcel,

On Thu, Jun 26, 2014 at 3:57 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andre,
>
>> The Bluetooth 4.1 introduces a new LE meta event called "LE Remote
>> Connection Parameter Request" event. In order to the controller sends
>> this event to host, we should enable it during controller initialization.
>>
>> Therefore, this patch enables that event in case the controller supports
>> Bluetooth 4.1 or upper version.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> include/net/bluetooth/bluetooth.h | 1 +
>> net/bluetooth/hci_core.c          | 7 +++++++
>> 2 files changed, 8 insertions(+)
>>
>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
>> index 373000d..85dbc03 100644
>> --- a/include/net/bluetooth/bluetooth.h
>> +++ b/include/net/bluetooth/bluetooth.h
>> @@ -38,6 +38,7 @@
>> #define BLUETOOTH_VER_1_1     1
>> #define BLUETOOTH_VER_1_2     2
>> #define BLUETOOTH_VER_2_0     3
>> +#define BLUETOOTH_VER_4_1    7
>>
>> /* Reserv for core and drivers use */
>> #define BT_SKB_RESERVE        8
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 9852449..a8b9d70 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -1461,6 +1461,13 @@ static void hci_setup_event_mask(struct hci_request *req)
>>       if (lmp_le_capable(hdev)) {
>>               memset(events, 0, sizeof(events));
>>               events[0] = 0x1f;
>> +
>> +             /* If Bluetooth 4.1 or upper version, enable LE Remote
>> +              * Connection Parameter Request Event.
>> +              */
>> +             if (hdev->hci_ver >= BLUETOOTH_VER_4_1)
>> +                     events[0] = events[0] | 0x20;
>> +
>
> we only fall back to Bluetooth version check in case we can not feature masks to determine if something is supported or not. Use the LE features mask to figure out if we support this or not.

Ok, I'll fix it.

Thanks,

Andre

Re: [RFC v2 4/4] Bluetooth: Introduce "New Connection Parameter" Event

[Andre Guedes]

Hi Marcel,

On Thu, Jun 26, 2014 at 4:00 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andre,
>
>> This patch introduces a new Mgmt event called "New Connection Parameter".
>> This event indicates to userspace the connection parameters values the
>> remote device requested.
>>
>> The user may store these values and load them into kernel (an upcoming
>> patch will introduce a Mgmt command to do that). This way, next time a
>> connection is established to that device, the kernel will use those
>> parameters values instead of the default ones.
>>
>> This event is sent when the remote device requests new connection
>> parameters through Link Layer or L2CAP connection parameter update
>> procedures.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> include/net/bluetooth/hci_core.h |  3 +++
>> include/net/bluetooth/mgmt.h     | 10 ++++++++++
>> net/bluetooth/hci_event.c        |  3 +++
>> net/bluetooth/l2cap_core.c       |  6 +++++-
>> net/bluetooth/mgmt.c             | 19 +++++++++++++++++++
>> 5 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index 016d262..f318efe 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -1322,6 +1322,9 @@ void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent);
>> void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk);
>> void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
>>                  bool persistent);
>> +void mgmt_new_conn_param(struct hci_dev *hdev, bdaddr_t *bdaddr,
>> +                      u8 bdaddr_type, u16 min_interval, u16 max_interval,
>> +                      u16 latency, u16 timeout);
>> void mgmt_reenable_advertising(struct hci_dev *hdev);
>> void mgmt_smp_complete(struct hci_conn *conn, bool complete);
>>
>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
>> index bcffc9a..de22982 100644
>> --- a/include/net/bluetooth/mgmt.h
>> +++ b/include/net/bluetooth/mgmt.h
>> @@ -578,3 +578,13 @@ struct mgmt_ev_new_csrk {
>>       __u8 store_hint;
>>       struct mgmt_csrk_info key;
>> } __packed;
>> +
>> +#define MGMT_EV_NEW_CONN_PARAM               0x001a
>> +struct mgmt_ev_new_conn_param {
>> +     struct mgmt_addr_info addr;
>> +     __u8 store_hint;
>> +     __le16 min_interval;
>> +     __le16 max_interval;
>> +     __le16 latency;
>> +     __le16 timeout;
>> +} __packed;
>
> the only open question I still have is if we want to add a __u8 master variable here. The connection parameters are no longer strictly speaking slave to master only. With link layer topology, there are a bit more flexible.

If we are the slave side and the master requests a parameters update,
I don't see why we should store the new parameters. The connection is
always initiated by the master side and, in this case, it is
responsible for choosing the connection parameters.

We may send the "New Connection Parameter" event just to let the
user-space knows the parameters have changed, but it would always be
store_hint = 0x00.

> In addition it could be possible that we also act as slave and in that case we request the parameters. I wonder if that should be done via this event as well or if we just use socket option for that. Thoughts?

I'm not sure about this, but it seems a socket option is a good way to
request new connection parameters.

Regards,

Andre

Re: [RFC v2 4/4] Bluetooth: Introduce "New Connection Parameter" Event

[Marcel Holtmann]

Hi Andre,

>>> This patch introduces a new Mgmt event called "New Connection Parameter".
>>> This event indicates to userspace the connection parameters values the
>>> remote device requested.
>>> 
>>> The user may store these values and load them into kernel (an upcoming
>>> patch will introduce a Mgmt command to do that). This way, next time a
>>> connection is established to that device, the kernel will use those
>>> parameters values instead of the default ones.
>>> 
>>> This event is sent when the remote device requests new connection
>>> parameters through Link Layer or L2CAP connection parameter update
>>> procedures.
>>> 
>>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>>> ---
>>> include/net/bluetooth/hci_core.h |  3 +++
>>> include/net/bluetooth/mgmt.h     | 10 ++++++++++
>>> net/bluetooth/hci_event.c        |  3 +++
>>> net/bluetooth/l2cap_core.c       |  6 +++++-
>>> net/bluetooth/mgmt.c             | 19 +++++++++++++++++++
>>> 5 files changed, 40 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>> index 016d262..f318efe 100644
>>> --- a/include/net/bluetooth/hci_core.h
>>> +++ b/include/net/bluetooth/hci_core.h
>>> @@ -1322,6 +1322,9 @@ void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent);
>>> void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk);
>>> void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk,
>>>                 bool persistent);
>>> +void mgmt_new_conn_param(struct hci_dev *hdev, bdaddr_t *bdaddr,
>>> +                      u8 bdaddr_type, u16 min_interval, u16 max_interval,
>>> +                      u16 latency, u16 timeout);
>>> void mgmt_reenable_advertising(struct hci_dev *hdev);
>>> void mgmt_smp_complete(struct hci_conn *conn, bool complete);
>>> 
>>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
>>> index bcffc9a..de22982 100644
>>> --- a/include/net/bluetooth/mgmt.h
>>> +++ b/include/net/bluetooth/mgmt.h
>>> @@ -578,3 +578,13 @@ struct mgmt_ev_new_csrk {
>>>      __u8 store_hint;
>>>      struct mgmt_csrk_info key;
>>> } __packed;
>>> +
>>> +#define MGMT_EV_NEW_CONN_PARAM               0x001a
>>> +struct mgmt_ev_new_conn_param {
>>> +     struct mgmt_addr_info addr;
>>> +     __u8 store_hint;
>>> +     __le16 min_interval;
>>> +     __le16 max_interval;
>>> +     __le16 latency;
>>> +     __le16 timeout;
>>> +} __packed;
>> 
>> the only open question I still have is if we want to add a __u8 master variable here. The connection parameters are no longer strictly speaking slave to master only. With link layer topology, there are a bit more flexible.
> 
> If we are the slave side and the master requests a parameters update,
> I don't see why we should store the new parameters. The connection is
> always initiated by the master side and, in this case, it is
> responsible for choosing the connection parameters.

good point. And if we say that when we are slave, the connection parameter update is triggered via socket option, then there is really nothing to do here.

> We may send the "New Connection Parameter" event just to let the
> user-space knows the parameters have changed, but it would always be
> store_hint = 0x00.

Lets not send them then. When we are slave, we can check them for ourselves from the socket option that we are using to set them. So no need to send them around.

Also it would be confusing when you switch between master and slave roles. Then you do not know which ones are which. So not sending them is better here.

>> In addition it could be possible that we also act as slave and in that case we request the parameters. I wonder if that should be done via this event as well or if we just use socket option for that. Thoughts?
> 
> I'm not sure about this, but it seems a socket option is a good way to
> request new connection parameters.

I think socket option is best solution here.

Regards

Marcel