2012-07-27 18:10:10

by Andre Guedes

[permalink] [raw]
Subject: [PATCH 1/7] Bluetooth: Trivial refactoring

This patch replaces the unlock-and-return statements by the goto
statement.

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_event.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0386e1e..e89d7f2 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3364,8 +3364,7 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
conn = hci_conn_add(hdev, LE_LINK, &ev->bdaddr);
if (!conn) {
BT_ERR("No memory for new connection");
- hci_dev_unlock(hdev);
- return;
+ goto unlock;
}

conn->dst_type = ev->bdaddr_type;
--
1.7.11.2



2012-07-27 18:10:16

by Andre Guedes

[permalink] [raw]
Subject: [PATCH 7/7] Bluetooth: Refactor in hci_le_conn_complete_evt

This patch moves the hci_conn check to begining of hci_le_conn_
complete_evt in order to improve code's readability and better
error handling.

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_event.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 92522b4..32e21ad 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3334,19 +3334,6 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
hci_dev_lock(hdev);

conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
-
- if (ev->status) {
- if (!conn)
- goto unlock;
-
- mgmt_connect_failed(hdev, &conn->dst, conn->type,
- conn->dst_type, ev->status);
- hci_proto_connect_cfm(conn, ev->status);
- conn->state = BT_CLOSED;
- hci_conn_del(conn);
- goto unlock;
- }
-
if (!conn) {
conn = hci_conn_add(hdev, LE_LINK, &ev->bdaddr);
if (!conn) {
@@ -3362,6 +3349,15 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
}
}

+ if (ev->status) {
+ mgmt_connect_failed(hdev, &conn->dst, conn->type,
+ conn->dst_type, ev->status);
+ hci_proto_connect_cfm(conn, ev->status);
+ conn->state = BT_CLOSED;
+ hci_conn_del(conn);
+ goto unlock;
+ }
+
if (!test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags))
mgmt_device_connected(hdev, &ev->bdaddr, conn->type,
conn->dst_type, 0, NULL, 0, NULL);
--
1.7.11.2


2012-07-27 18:10:15

by Andre Guedes

[permalink] [raw]
Subject: [PATCH 6/7] Bluetooth: Lookup hci_conn in hci_le_conn_complete_evt

This patch does a trivial code refactoring in hci_conn lookup in
hci_le_conn_complete_evt. It performs the hci_conn lookup at the
begining of the function since it is used by both flows (error
and success).

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_event.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8dc1f0f..92522b4 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3333,8 +3333,9 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)

hci_dev_lock(hdev);

+ conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
+
if (ev->status) {
- conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
if (!conn)
goto unlock;

@@ -3346,7 +3347,6 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
goto unlock;
}

- conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &ev->bdaddr);
if (!conn) {
conn = hci_conn_add(hdev, LE_LINK, &ev->bdaddr);
if (!conn) {
--
1.7.11.2


2012-07-27 18:10:14

by Andre Guedes

[permalink] [raw]
Subject: [PATCH 5/7] Bluetooth: Find hci_conn by BT_CONNECT state

This patch changes hci_cs_le_create_conn to perform hci_conn lookup
by state instead of bdaddr.

Since we can have only one LE connection in BT_CONNECT state, we can
perform LE hci_conn lookup by state. This way, we don't rely on
hci_sent_cmd_data helper to find the hci_conn object.

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_event.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index c0aa9f4..8dc1f0f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1614,29 +1614,24 @@ static void hci_cs_disconnect(struct hci_dev *hdev, u8 status)

static void hci_cs_le_create_conn(struct hci_dev *hdev, __u8 status)
{
- struct hci_cp_le_create_conn *cp;
struct hci_conn *conn;

BT_DBG("%s status 0x%2.2x", hdev->name, status);

- cp = hci_sent_cmd_data(hdev, HCI_OP_LE_CREATE_CONN);
- if (!cp)
- return;
-
if (status) {
hci_dev_lock(hdev);

- conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->peer_addr);
+ conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
if (!conn) {
hci_dev_unlock(hdev);
return;
}

- BT_DBG("%s bdaddr %s conn %p", hdev->name, batostr(&cp->peer_addr),
+ BT_DBG("%s bdaddr %s conn %p", hdev->name, batostr(&conn->dst),
conn);

conn->state = BT_CLOSED;
- mgmt_connect_failed(hdev, &cp->peer_addr, conn->type,
+ mgmt_connect_failed(hdev, &conn->dst, conn->type,
conn->dst_type, status);
hci_proto_connect_cfm(conn, status);
hci_conn_del(conn);
--
1.7.11.2


2012-07-27 18:10:13

by Andre Guedes

[permalink] [raw]
Subject: [PATCH 4/7] Bluetooth: Refactor hci_cs_le_create_conn

This patch does some code refactoring in hci_cs_le_create_conn
function. The hci_conn object is only needed in case of failure,
therefore hdev locking and hci_conn lookup were moved to
if-statement scope.

Also, the conn->state check was removed since we should always
close the connection if it fails.

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_event.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 27064be..c0aa9f4 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1623,24 +1623,26 @@ static void hci_cs_le_create_conn(struct hci_dev *hdev, __u8 status)
if (!cp)
return;

- hci_dev_lock(hdev);
+ if (status) {
+ hci_dev_lock(hdev);

- conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->peer_addr);
+ conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->peer_addr);
+ if (!conn) {
+ hci_dev_unlock(hdev);
+ return;
+ }

- BT_DBG("%s bdaddr %s conn %p", hdev->name, batostr(&cp->peer_addr),
- conn);
+ BT_DBG("%s bdaddr %s conn %p", hdev->name, batostr(&cp->peer_addr),
+ conn);

- if (status) {
- if (conn && conn->state == BT_CONNECT) {
- conn->state = BT_CLOSED;
- mgmt_connect_failed(hdev, &cp->peer_addr, conn->type,
- conn->dst_type, status);
- hci_proto_connect_cfm(conn, status);
- hci_conn_del(conn);
- }
- }
+ conn->state = BT_CLOSED;
+ mgmt_connect_failed(hdev, &cp->peer_addr, conn->type,
+ conn->dst_type, status);
+ hci_proto_connect_cfm(conn, status);
+ hci_conn_del(conn);

- hci_dev_unlock(hdev);
+ hci_dev_unlock(hdev);
+ }
}

static void hci_cs_le_start_enc(struct hci_dev *hdev, u8 status)
--
1.7.11.2


2012-07-27 18:10:12

by Andre Guedes

[permalink] [raw]
Subject: [PATCH 3/7] Bluetooth: Remove unneeded code

This patch removes some unneeded code from hci_cs_le_create_conn.

If the hci_conn is not found, it means this LE connection attempt
was triggered by a thrid-party tool (e.g. hcitool). We should not
create this new hci_conn in LE Create Connection command status
event since it is already properly handled in LE Connection
Complete event.

Signed-off-by: Andre Guedes <[email protected]>
---
net/bluetooth/hci_event.c | 10 ----------
1 file changed, 10 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8b13ccc..27064be 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1638,16 +1638,6 @@ static void hci_cs_le_create_conn(struct hci_dev *hdev, __u8 status)
hci_proto_connect_cfm(conn, status);
hci_conn_del(conn);
}
- } else {
- if (!conn) {
- conn = hci_conn_add(hdev, LE_LINK, &cp->peer_addr);
- if (conn) {
- conn->dst_type = cp->peer_addr_type;
- conn->out = true;
- } else {
- BT_ERR("No memory for new connection");
- }
- }
}

hci_dev_unlock(hdev);
--
1.7.11.2


2012-07-27 18:10:11

by Andre Guedes

[permalink] [raw]
Subject: [PATCH 2/7] Bluetooth: Fix hci_le_conn_complete_evt

We need to check the 'Role' parameter from the LE Connection
Complete Event in order to properly set 'out' and 'link_mode'
fields from hci_conn structure.

Signed-off-by: Andre Guedes <[email protected]>
---
include/net/bluetooth/hci.h | 2 ++
net/bluetooth/hci_event.c | 5 +++++
2 files changed, 7 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 7f19556..23cf413 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1304,6 +1304,8 @@ struct hci_ev_num_comp_blocks {
} __packed;

/* Low energy meta events */
+#define LE_CONN_ROLE_MASTER 0x00
+
#define HCI_EV_LE_CONN_COMPLETE 0x01
struct hci_ev_le_conn_complete {
__u8 status;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index e89d7f2..8b13ccc 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3368,6 +3368,11 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
}

conn->dst_type = ev->bdaddr_type;
+
+ if (ev->role == LE_CONN_ROLE_MASTER) {
+ conn->out = true;
+ conn->link_mode |= HCI_LM_MASTER;
+ }
}

if (!test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags))
--
1.7.11.2


2012-08-06 18:16:12

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 7/7] Bluetooth: Refactor in hci_le_conn_complete_evt

Hi Andre,

* Andre Guedes <[email protected]> [2012-07-27 15:10:16 -0300]:

> This patch moves the hci_conn check to begining of hci_le_conn_
> complete_evt in order to improve code's readability and better
> error handling.
>
> Signed-off-by: Andre Guedes <[email protected]>
> ---
> net/bluetooth/hci_event.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)

All seven patches have been applied to bluetooth-next, Thanks.

Gustavo