[RFC 0/4] New Connection Parameter Event

[RFC 0/4] New Connection Parameter Event

[Andre Guedes]

Hi all,

This small RFC patch set introduces the "New Connection Parameter" event as
described in mgmt-api.txt.

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

Patch 2 adds support for LE Connection Parameters Request Link Layer control
procedure introduced in Core spec 4.1. I was not able to test this specific
patch since I have no Bluetooth 4.1 dongle. It would be nice someone else test
it before applying the patch. If no one is able to test it, I can remove it
from the patch set and re-send it later.

Regards,

Andre

Andre Guedes (4):
  Bluetooth: Connection parameters check helper
  Bluetooth: Connection Parameter Update Procedure
  Bluetooth: Move is_identity_address() to hci_core.h
  Bluetooth: Introduce "New Connection Parameter" Event

 include/net/bluetooth/hci.h      | 28 +++++++++++++++++++++
 include/net/bluetooth/hci_core.h | 36 +++++++++++++++++++++++++++
 include/net/bluetooth/mgmt.h     | 10 ++++++++
 net/bluetooth/hci_core.c         | 14 +----------
 net/bluetooth/hci_event.c        | 53 ++++++++++++++++++++++++++++++++++++++++
 net/bluetooth/l2cap_core.c       | 29 +++++-----------------
 net/bluetooth/mgmt.c             | 34 ++++++++++++++++++++++++++
 7 files changed, 168 insertions(+), 36 deletions(-)

-- 
1.9.1

[RFC 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 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 3/4] Bluetooth: Move is_identity_address() to hci_core.h

[Andre Guedes]

This patch adds the prefix hci_ to is_identity_address() helper and
moves it to hci_core.h so it can be reused in other files.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 016d262..b6b1c86 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1164,6 +1164,18 @@ static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type)
 	return false;
 }
 
+static inline bool hci_is_identity_address(bdaddr_t *addr, u8 addr_type)
+{
+	if (addr_type == ADDR_LE_DEV_PUBLIC)
+		return true;
+
+	/* Check for Random Static address type */
+	if ((addr->b[5] & 0xc0) == 0xc0)
+		return true;
+
+	return false;
+}
+
 static inline struct smp_irk *hci_get_irk(struct hci_dev *hdev,
 					  bdaddr_t *bdaddr, u8 addr_type)
 {
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9852449..b8b901c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3369,18 +3369,6 @@ static bool is_connected(struct hci_dev *hdev, bdaddr_t *addr, u8 type)
 	return true;
 }
 
-static bool is_identity_address(bdaddr_t *addr, u8 addr_type)
-{
-	if (addr_type == ADDR_LE_DEV_PUBLIC)
-		return true;
-
-	/* Check for Random Static address type */
-	if ((addr->b[5] & 0xc0) == 0xc0)
-		return true;
-
-	return false;
-}
-
 /* This function requires the caller holds hdev->lock */
 int hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
 			u8 auto_connect, u16 conn_min_interval,
@@ -3388,7 +3376,7 @@ int hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
 {
 	struct hci_conn_params *params;
 
-	if (!is_identity_address(addr, addr_type))
+	if (!hci_is_identity_address(addr, addr_type))
 		return -EINVAL;
 
 	params = hci_conn_params_lookup(hdev, addr, addr_type);
-- 
1.9.1

[RFC 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             | 34 ++++++++++++++++++++++++++++++++++
 5 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b6b1c86..d792a1e 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1334,6 +1334,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..708cc0a 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,39 @@ 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;
+	struct smp_irk *irk;
+
+	memset(&ev, 0, sizeof(ev));
+
+	irk = hci_get_irk(hdev, bdaddr, bdaddr_type);
+	if (irk) {
+		bacpy(&ev.addr.bdaddr, &irk->bdaddr);
+		ev.addr.type = link_to_bdaddr(LE_LINK, irk->addr_type);
+
+		ev.store_hint = 0x01;
+	} else {
+		bacpy(&ev.addr.bdaddr, bdaddr);
+		ev.addr.type = link_to_bdaddr(LE_LINK, bdaddr_type);
+
+		if (hci_is_identity_address(bdaddr, bdaddr_type))
+			ev.store_hint = 0x01;
+		else
+			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 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             | 34 ++++++++++++++++++++++++++++++++++
> 5 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index b6b1c86..d792a1e 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1334,6 +1334,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..708cc0a 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,39 @@ 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;
> +	struct smp_irk *irk;
> +
> +	memset(&ev, 0, sizeof(ev));
> +
> +	irk = hci_get_irk(hdev, bdaddr, bdaddr_type);
> +	if (irk) {
> +		bacpy(&ev.addr.bdaddr, &irk->bdaddr);
> +		ev.addr.type = link_to_bdaddr(LE_LINK, irk->addr_type);
> +
> +		ev.store_hint = 0x01;
> +	} else {
> +		bacpy(&ev.addr.bdaddr, bdaddr);
> +		ev.addr.type = link_to_bdaddr(LE_LINK, bdaddr_type);
> +
> +		if (hci_is_identity_address(bdaddr, bdaddr_type))
> +			ev.store_hint = 0x01;
> +		else
> +			ev.store_hint = 0x00;
> +	}

this whole IRK check and matching it up to store_hint makes no sense. Just start out with having store_hint set to 0x00 and we fix the correct handling for that part later.

Regards

Marcel

Re: [RFC 3/4] Bluetooth: Move is_identity_address() to hci_core.h

[Marcel Holtmann]

Hi Andre,

> This patch adds the prefix hci_ to is_identity_address() helper and
> moves it to hci_core.h so it can be reused in other files.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h | 12 ++++++++++++
> net/bluetooth/hci_core.c         | 14 +-------------
> 2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 016d262..b6b1c86 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1164,6 +1164,18 @@ static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type)
> 	return false;
> }
> 
> +static inline bool hci_is_identity_address(bdaddr_t *addr, u8 addr_type)
> +{
> +	if (addr_type == ADDR_LE_DEV_PUBLIC)
> +		return true;
> +
> +	/* Check for Random Static address type */
> +	if ((addr->b[5] & 0xc0) == 0xc0)
> +		return true;
> +
> +	return false;
> +}
> +
> static inline struct smp_irk *hci_get_irk(struct hci_dev *hdev,
> 					  bdaddr_t *bdaddr, u8 addr_type)
> {
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9852449..b8b901c 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3369,18 +3369,6 @@ static bool is_connected(struct hci_dev *hdev, bdaddr_t *addr, u8 type)
> 	return true;
> }
> 
> -static bool is_identity_address(bdaddr_t *addr, u8 addr_type)
> -{
> -	if (addr_type == ADDR_LE_DEV_PUBLIC)
> -		return true;
> -
> -	/* Check for Random Static address type */
> -	if ((addr->b[5] & 0xc0) == 0xc0)
> -		return true;
> -
> -	return false;
> -}
> -
> /* This function requires the caller holds hdev->lock */
> int hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
> 			u8 auto_connect, u16 conn_min_interval,
> @@ -3388,7 +3376,7 @@ int hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
> {
> 	struct hci_conn_params *params;
> 
> -	if (!is_identity_address(addr, addr_type))
> +	if (!hci_is_identity_address(addr, addr_type))
> 		return -EINVAL;

I do not really get the logic here. Why do we need to do this.

Regards

Marcel

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

[Marcel Holtmann]

Hi Andre,

> 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;
> +

you also need a patch that will enable this event.

Regards

Marcel

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

[Andre Guedes]

Hi Marcel,

On Wed, Jun 25, 2014 at 5:18 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andre,
>
>> 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;
>> +
>
> you also need a patch that will enable this event.

Yes, I've missed that. I'll add a separate patch to enable this event.

Thanks,

Andre

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

[Andre Guedes]

Hi Marcel,

On Wed, Jun 25, 2014 at 5:14 PM, 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             | 34 ++++++++++++++++++++++++++++++++++
>> 5 files changed, 55 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index b6b1c86..d792a1e 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -1334,6 +1334,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..708cc0a 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,39 @@ 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;
>> +     struct smp_irk *irk;
>> +
>> +     memset(&ev, 0, sizeof(ev));
>> +
>> +     irk = hci_get_irk(hdev, bdaddr, bdaddr_type);
>> +     if (irk) {
>> +             bacpy(&ev.addr.bdaddr, &irk->bdaddr);
>> +             ev.addr.type = link_to_bdaddr(LE_LINK, irk->addr_type);
>> +
>> +             ev.store_hint = 0x01;
>> +     } else {
>> +             bacpy(&ev.addr.bdaddr, bdaddr);
>> +             ev.addr.type = link_to_bdaddr(LE_LINK, bdaddr_type);
>> +
>> +             if (hci_is_identity_address(bdaddr, bdaddr_type))
>> +                     ev.store_hint = 0x01;
>> +             else
>> +                     ev.store_hint = 0x00;
>> +     }
>
> this whole IRK check and matching it up to store_hint makes no sense. Just start out with having store_hint set to 0x00 and we fix the correct handling for that part later.

Ok, I'll change this and always set store_hint to 0x00 for now.

Regards,

Andre

Re: [RFC 3/4] Bluetooth: Move is_identity_address() to hci_core.h

[Andre Guedes]

Hi Marcel,

On Wed, Jun 25, 2014 at 5:16 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andre,
>
>> This patch adds the prefix hci_ to is_identity_address() helper and
>> moves it to hci_core.h so it can be reused in other files.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> include/net/bluetooth/hci_core.h | 12 ++++++++++++
>> net/bluetooth/hci_core.c         | 14 +-------------
>> 2 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index 016d262..b6b1c86 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -1164,6 +1164,18 @@ static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type)
>>       return false;
>> }
>>
>> +static inline bool hci_is_identity_address(bdaddr_t *addr, u8 addr_type)
>> +{
>> +     if (addr_type == ADDR_LE_DEV_PUBLIC)
>> +             return true;
>> +
>> +     /* Check for Random Static address type */
>> +     if ((addr->b[5] & 0xc0) == 0xc0)
>> +             return true;
>> +
>> +     return false;
>> +}
>> +
>> static inline struct smp_irk *hci_get_irk(struct hci_dev *hdev,
>>                                         bdaddr_t *bdaddr, u8 addr_type)
>> {
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 9852449..b8b901c 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -3369,18 +3369,6 @@ static bool is_connected(struct hci_dev *hdev, bdaddr_t *addr, u8 type)
>>       return true;
>> }
>>
>> -static bool is_identity_address(bdaddr_t *addr, u8 addr_type)
>> -{
>> -     if (addr_type == ADDR_LE_DEV_PUBLIC)
>> -             return true;
>> -
>> -     /* Check for Random Static address type */
>> -     if ((addr->b[5] & 0xc0) == 0xc0)
>> -             return true;
>> -
>> -     return false;
>> -}
>> -
>> /* This function requires the caller holds hdev->lock */
>> int hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
>>                       u8 auto_connect, u16 conn_min_interval,
>> @@ -3388,7 +3376,7 @@ int hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
>> {
>>       struct hci_conn_params *params;
>>
>> -     if (!is_identity_address(addr, addr_type))
>> +     if (!hci_is_identity_address(addr, addr_type))
>>               return -EINVAL;
>
> I do not really get the logic here. Why do we need to do this.

Forget about this patch. It became pointless after your comment on patch 4.

Regards,

Andre