linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RE: LMP transaction collision at Set encryption
@ 2012-02-13  6:22 Mohanan, Rajmohan
  2012-02-13  7:28 ` Marcel Holtmann
  0 siblings, 1 reply; 4+ messages in thread
From: Mohanan, Rajmohan @ 2012-02-13  6:22 UTC (permalink / raw)
  To: linux-bluetooth@vger.kernel.org
  Cc: Holtmann, Marcel, linux-bluetooth@vger.kernel.org,
	padovan@profusion.mobi, marcel@holtmann.org

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


Hi,
             I got an interesting issue in bluez stack .

1.       I started pairing from my device(DUT) to a remote device (Lenovo T500). After successful bonding bluez send device discovery in    the same ACL connection created prior to BONDING
2.       I'm changed to my role as slave.
3.       From my LMP ,sending Set Connection Encryption, I'm getting LMP Error Transaction Collision as status of my encryption command
           (Remote guy who is a master has also initiated Set encryption).
4.       In between bluez has initiated SDP search after bonding process complete(device_bonding_complete()) so l2cap connect req has initiated and the BT state is BT_CONFIG
5.       From the encryption change event (event status is )x23(LMP transaction collision),l2cap_connect_cfm will call and in that we are disconnecting l2cap and then acl link.

 If we are getting Lmp transaction collision and we are slave . do we need to Disconnect ACL link ? .

We are not able Find the services of remote device because application written in spite of service discovery has initiated after bonding process.


I'm attaching  Hci dump and Air trace
>From  hci dump You can find that Slave(DUT) is sending Set encryption command and From Air trace you can see Encryption mod request from Master.
 
I took air trace with Link key which is generated previously. So initial LMP transaction I got in air trace. 
I could not able to put new link key which is generated in current transaction, so after encryption the air trace has some missing packets You can see after  frame 3288 in air trace.


/*code changes*/
                      I have  made changes in hci_event.c  for solving LMP Transaction collision.
When we gets Encrypt change event with error code as LMP  transaction collision , I'm ignoring the change event because From Master Encrypt change event will process and will get encrypt change event with success second time. If we are not getting Encrypt change event from master we are sending again Set encryption from slave( because we already sent a set encryption which result in to a collision) after 1 second delay. If we getting a encrypt change event from master after collision event then we delete timer and process it normally.

I got another issue also  with Lenovo T500laptop where , when ever after bonding Lenovo T500 is initiating a SDP search Request which collides our SDP request(from Bluez we are initiator we are starting SDP query immediately and if we are responder we are delaying for 2 seconds for starting SDP query).

 
For solving this IOP issue I have made a change that if we are initiator also we will delay for 1 second and starting SDP query after bonding.


I'm attaching patch for above two issues.

Please review it and  let me know the feedback.

Regards
Rajmohan




[-- Attachment #2: 0001-LMP-Transaction-Collision-handling-set-encryption.patch --]
[-- Type: application/octet-stream, Size: 6503 bytes --]

From 957ce9ab97a55b2522abc5b5ab9dfbeed9432dc1 Mon Sep 17 00:00:00 2001
From: mohanan <rajmohan.mohanan@intel.com>
Date: Fri, 10 Feb 2012 09:56:23 +0530
Subject: [PATCH] LMP Transaction Collision handling[set encryption]

Change-Id: Ibaa5361fa782efb27acaff0c4433ad2901faeca1
---
 include/linux/kernel.h           |    5 +++--
 include/net/bluetooth/hci.h      |    1 +
 include/net/bluetooth/hci_core.h |   21 +++++++++++----------
 net/bluetooth/hci_conn.c         |   22 +++++++++++++++++++++-
 net/bluetooth/hci_event.c        |   16 ++++++++++++----
 5 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index f4e3184..75e7f89 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -20,6 +20,7 @@
 #include <asm/byteorder.h>
 #include <asm/bug.h>
 
+
 extern const char linux_banner[];
 extern const char linux_proc_banner[];
 
@@ -386,7 +387,7 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
 /* pr_devel() should produce zero code unless DEBUG is defined */
 #ifdef DEBUG
 #define pr_devel(fmt, ...) \
-	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+	printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
 #else
 #define pr_devel(fmt, ...) \
 	({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); 0; })
@@ -395,7 +396,7 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
 /* If you are writing a driver, please use dev_dbg instead */
 #if defined(DEBUG)
 #define pr_debug(fmt, ...) \
-	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+	printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
 #elif defined(CONFIG_DYNAMIC_DEBUG)
 /* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
 #define pr_debug(fmt, ...) do { \
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index a60b694..4f20612 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -104,6 +104,7 @@ enum {
 #define HCI_PAIRING_TIMEOUT	(60000)	/* 60 seconds */
 #define HCI_IDLE_TIMEOUT	(6000)	/* 6 seconds */
 #define HCI_INIT_TIMEOUT	(10000)	/* 10 seconds */
+#define HCI_ENCRYPTION_TIMEOUT (1000) /* 1 Seconds  */
 
 /* HCI data types */
 #define HCI_COMMAND_PKT		0x01
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index d9b20bc..7bef2ce 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -187,16 +187,17 @@ struct hci_conn {
 	__u8		 out;
 	__u8		 attempt;
 	__u8		 dev_class[3];
-	__u8             features[8];
-	__u8             ssp_mode;
-	__u16            interval;
-	__u16            pkt_type;
-	__u16            link_policy;
+	__u8         features[8];
+	__u8         ssp_mode;
+	__u16        interval;
+	__u16        pkt_type;
+	__u16        link_policy;
 	__u32		 link_mode;
-	__u8             auth_type;
-	__u8             sec_level;
-	__u8             power_save;
-	__u16            disc_timeout;
+	__u8         auth_type;
+	__u8         sec_level;
+	__u8         power_save;
+	__u16        disc_timeout;
+	__u16        encrypt_timeout;   
 	unsigned long	 pend;
 
 	unsigned int	 sent;
@@ -205,7 +206,7 @@ struct hci_conn {
 
 	struct timer_list disc_timer;
 	struct timer_list idle_timer;
-
+    struct timer_list encrypt_timer;
 	struct work_struct work_add;
 	struct work_struct work_del;
 
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 2f4d30f..22a6df0 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -195,7 +195,23 @@ static void hci_conn_idle(unsigned long arg)
 
 	hci_conn_enter_sniff_mode(conn);
 }
+static void hci_conn_encryption(unsigned long arg)
+{
+	struct hci_conn *conn = (void *) arg;
+    
+	BT_DBG("Encryption status check");
 
+	if((conn) && (test_and_clear_bit(HCI_CONN_ENCRYPT_PEND,&conn->pend)))
+	{
+		struct hci_dev *hdev = conn->hdev;
+		del_timer(&conn->encrypt_timer);
+		struct hci_cp_set_conn_encrypt cp;
+		cp.handle  = conn->handle;
+		cp.encrypt = 0x01;
+		hci_send_cmd(hdev, HCI_OP_SET_CONN_ENCRYPT,
+					sizeof(cp), &cp);
+	}
+}
 struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type,
 					__u16 pkt_type, bdaddr_t *dst)
 {
@@ -216,6 +232,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type,
 
 	conn->power_save = 1;
 	conn->disc_timeout = HCI_DISCONN_TIMEOUT;
+	conn->encrypt_timeout = HCI_ENCRYPTION_TIMEOUT;
 
 	switch (type) {
 	case ACL_LINK:
@@ -245,6 +262,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type,
 
 	setup_timer(&conn->disc_timer, hci_conn_timeout, (unsigned long)conn);
 	setup_timer(&conn->idle_timer, hci_conn_idle, (unsigned long)conn);
+	setup_timer(&conn->encrypt_timer, hci_conn_encryption, (unsigned long)conn);
 
 	atomic_set(&conn->refcnt, 0);
 
@@ -275,6 +293,8 @@ int hci_conn_del(struct hci_conn *conn)
 
 	del_timer(&conn->disc_timer);
 
+	del_timer(&conn->encrypt_timer);
+
 	if (conn->type == ACL_LINK) {
 		struct hci_conn *sco = conn->link;
 		if (sco)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 92654a3..1a6c668 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -659,7 +659,7 @@ static void hci_cs_set_conn_encrypt(struct hci_dev *hdev, __u8 status)
 
 	if (!status)
 		return;
-
+       
 	cp = hci_sent_cmd_data(hdev, HCI_OP_SET_CONN_ENCRYPT);
 	if (!cp)
 		return;
@@ -1103,14 +1103,15 @@ static inline void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *
 {
 	struct hci_ev_encrypt_change *ev = (void *) skb->data;
 	struct hci_conn *conn;
-
+    unsigned long timeo;
 	BT_DBG("%s status %d", hdev->name, ev->status);
 
 	hci_dev_lock(hdev);
-
+    
 	conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(ev->handle));
 	if (conn) {
 		if (!ev->status) {
+			del_timer(&conn->encrypt_timer);
 			if (ev->encrypt) {
 				/* Encryption implies authentication */
 				conn->link_mode |= HCI_LM_AUTH;
@@ -1118,6 +1119,13 @@ static inline void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *
 			} else
 				conn->link_mode &= ~HCI_LM_ENCRYPT;
 		}
+	   else if(ev->status == 0x23)
+	   {
+	   		BT_DBG("LMP transactioon collision happened, we need to wait");
+			timeo = msecs_to_jiffies(conn->encrypt_timeout);
+		    mod_timer(&conn->encrypt_timer, jiffies + timeo);
+			goto done;
+	   }
 
 		clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend);
 
@@ -1130,7 +1138,7 @@ static inline void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *
 		} else
 			hci_encrypt_cfm(conn, ev->status, ev->encrypt);
 	}
-
+done:
 	hci_dev_unlock(hdev);
 }
 
-- 
1.7.0.4


[-- Attachment #3: 0001-Service-discovery-collision-after-bonding-handling.patch --]
[-- Type: application/octet-stream, Size: 1288 bytes --]

From aa3eb3b1ad7687e16b65bc7d1be68b06e14e6e9d Mon Sep 17 00:00:00 2001
From: mohanan <rajmohan.mohanan@intel.com>
Date: Fri, 10 Feb 2012 10:16:21 +0530
Subject: [PATCH] Service discovery collision after bonding handling

Change-Id: I0818451eb3cbcf64b5335e2ae624e6a872d86d34
---
 src/device.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/src/device.c b/src/device.c
index 5328db9..3793cb1 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2020,6 +2020,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t status)
 	 * before SDP. This is due to potential IOP issues if the other
 	 * end starts doing SDP at the same time as us */
 	if (bonding) {
+	#if 0
 		/* If we are initiators remove any discovery timer and just
 		 * start discovering services directly */
 		if (device->discov_timer) {
@@ -2029,7 +2030,13 @@ void device_bonding_complete(struct btd_device *device, uint8_t status)
 
 		device_browse(device, bonding->conn, bonding->msg,
 				NULL, FALSE);
-
+     #else
+	    debug("Service discovery started");
+			device->discov_timer = g_timeout_add(
+							1000,
+							start_discovery,
+							device);
+	 #endif
 		bonding_request_free(bonding);
 	} else {
 		if (!device->browse && !device->discov_timer &&
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-02-13 10:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-13  6:22 LMP transaction collision at Set encryption Mohanan, Rajmohan
2012-02-13  7:28 ` Marcel Holtmann
2012-02-13 10:16   ` Mohanan, Rajmohan
2012-02-13 10:41     ` Marcel Holtmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).