2012-08-09 07:52:27

by Mikel Astiz

[permalink] [raw]
Subject: [RFC v4 0/3] Disconnect reason in Kernel API

From: Mikel Astiz <[email protected]>

This patchset extends the management API to expose the low-level disconnect reason.

v4 integrates the feedback from Marcel, mainly:
- Fix minor coding style issue (patch v4 2/3)
- Remove references to "ACL" in mgmt terminology
- Add MGMT_DEV_DISCONN_UNKNOWN
- Add helper function to translate from HCI error codes to mgmt reason

Updated userland patch to be used for testing purposes will be submitted soon.

>From original patch:

During the BlueZ meeting in Brazil it was proposed to add two more values to this enum: "Connection terminated by local host" and "Connection terminated by remote host". However, after some testing, it seems the result can be quite misleading. Therefore and given that there are no known use-cases that need this information (local vs remote disconnection), these two values have been dropped.

Mikel Astiz (3):
Bluetooth: Add more HCI error codes
Bluetooth: Fix minor coding style in hci_event.c
Bluetooth: mgmt: Add device disconnect reason

include/net/bluetooth/hci.h | 3 +++
include/net/bluetooth/hci_core.h | 2 +-
include/net/bluetooth/mgmt.h | 9 +++++++++
net/bluetooth/hci_event.c | 32 ++++++++++++++++++++++++++------
net/bluetooth/mgmt.c | 9 +++++----
5 files changed, 44 insertions(+), 11 deletions(-)

--
1.7.7.6



2012-08-15 04:32:47

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [RFC v4 3/3] Bluetooth: mgmt: Add device disconnect reason

Hi Mikel,

* Mikel Astiz <[email protected]> [2012-08-09 09:52:30 +0200]:

> From: Mikel Astiz <[email protected]>
>
> MGMT_EV_DEVICE_DISCONNECTED will now expose the disconnection reason to
> userland, distinguishing four possible values:
>
> 0x00 Reason not known or unspecified
> 0x01 Connection timeout
> 0x02 Connection terminated by local host
> 0x03 Connection terminated by remote host
>
> Note that the local/remote distinction just determines which side
> terminated the low-level connection, regardless of the disconnection of
> the higher-level profiles.
>
> This can sometimes be misleading and thus must be used with care. For
> example, some hardware combinations would report a locally initiated
> disconnection even if the user turned Bluetooth off in the remote side.
>
> Signed-off-by: Mikel Astiz <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 2 +-
> include/net/bluetooth/mgmt.h | 9 +++++++++
> net/bluetooth/hci_event.c | 26 +++++++++++++++++++++++---
> net/bluetooth/mgmt.c | 9 +++++----
> 4 files changed, 38 insertions(+), 8 deletions(-)

All 3 patches have been applied to bluetooth-next. Thanks.

Gustavo

2012-08-09 15:50:58

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC v4 2/3] Bluetooth: Fix minor coding style in hci_event.c

Hi Mikel,

> Replace the status checks with the short form of the boolean expression.
>
> Signed-off-by: Mikel Astiz <[email protected]>
> ---
> net/bluetooth/hci_event.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-08-09 15:54:12

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC v4 3/3] Bluetooth: mgmt: Add device disconnect reason

Hi Mikel,

> MGMT_EV_DEVICE_DISCONNECTED will now expose the disconnection reason to
> userland, distinguishing four possible values:
>
> 0x00 Reason not known or unspecified
> 0x01 Connection timeout
> 0x02 Connection terminated by local host
> 0x03 Connection terminated by remote host
>
> Note that the local/remote distinction just determines which side
> terminated the low-level connection, regardless of the disconnection of
> the higher-level profiles.
>
> This can sometimes be misleading and thus must be used with care. For
> example, some hardware combinations would report a locally initiated
> disconnection even if the user turned Bluetooth off in the remote side.
>
> Signed-off-by: Mikel Astiz <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 2 +-
> include/net/bluetooth/mgmt.h | 9 +++++++++
> net/bluetooth/hci_event.c | 26 +++++++++++++++++++++++---
> net/bluetooth/mgmt.c | 9 +++++----
> 4 files changed, 38 insertions(+), 8 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-08-09 07:52:29

by Mikel Astiz

[permalink] [raw]
Subject: [RFC v4 2/3] Bluetooth: Fix minor coding style in hci_event.c

From: Mikel Astiz <[email protected]>

Replace the status checks with the short form of the boolean expression.

Signed-off-by: Mikel Astiz <[email protected]>
---
net/bluetooth/hci_event.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0386e1e..32fcc74 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -303,7 +303,7 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)

hci_dev_lock(hdev);

- if (status != 0) {
+ if (status) {
mgmt_write_scan_failed(hdev, param, status);
hdev->discov_timeout = 0;
goto done;
@@ -925,7 +925,7 @@ static void hci_cc_pin_code_reply(struct hci_dev *hdev, struct sk_buff *skb)
if (test_bit(HCI_MGMT, &hdev->dev_flags))
mgmt_pin_code_reply_complete(hdev, &rp->bdaddr, rp->status);

- if (rp->status != 0)
+ if (rp->status)
goto unlock;

cp = hci_sent_cmd_data(hdev, HCI_OP_PIN_CODE_REPLY);
@@ -1906,7 +1906,7 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)

if (test_and_clear_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags) &&
(conn->type == ACL_LINK || conn->type == LE_LINK)) {
- if (ev->status != 0)
+ if (ev->status)
mgmt_disconnect_failed(hdev, &conn->dst, conn->type,
conn->dst_type, ev->status);
else
@@ -3275,7 +3275,7 @@ static void hci_simple_pair_complete_evt(struct hci_dev *hdev,
* initiated the authentication. A traditional auth_complete
* event gets always produced as initiator and is also mapped to
* the mgmt_auth_failed event */
- if (!test_bit(HCI_CONN_AUTH_PEND, &conn->flags) && ev->status != 0)
+ if (!test_bit(HCI_CONN_AUTH_PEND, &conn->flags) && ev->status)
mgmt_auth_failed(hdev, &conn->dst, conn->type, conn->dst_type,
ev->status);

--
1.7.7.6


2012-08-09 07:52:28

by Mikel Astiz

[permalink] [raw]
Subject: [RFC v4 1/3] Bluetooth: Add more HCI error codes

From: Mikel Astiz <[email protected]>

Add more HCI error codes as defined in the specification.

Signed-off-by: Mikel Astiz <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/hci.h | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 7f19556..7253bdd 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -302,8 +302,11 @@ enum {

/* ---- HCI Error Codes ---- */
#define HCI_ERROR_AUTH_FAILURE 0x05
+#define HCI_ERROR_CONNECTION_TIMEOUT 0x08
#define HCI_ERROR_REJ_BAD_ADDR 0x0f
#define HCI_ERROR_REMOTE_USER_TERM 0x13
+#define HCI_ERROR_REMOTE_LOW_RESOURCES 0x14
+#define HCI_ERROR_REMOTE_POWER_OFF 0x15
#define HCI_ERROR_LOCAL_HOST_TERM 0x16
#define HCI_ERROR_PAIRING_NOT_ALLOWED 0x18

--
1.7.7.6


2012-08-09 07:52:30

by Mikel Astiz

[permalink] [raw]
Subject: [RFC v4 3/3] Bluetooth: mgmt: Add device disconnect reason

From: Mikel Astiz <[email protected]>

MGMT_EV_DEVICE_DISCONNECTED will now expose the disconnection reason to
userland, distinguishing four possible values:

0x00 Reason not known or unspecified
0x01 Connection timeout
0x02 Connection terminated by local host
0x03 Connection terminated by remote host

Note that the local/remote distinction just determines which side
terminated the low-level connection, regardless of the disconnection of
the higher-level profiles.

This can sometimes be misleading and thus must be used with care. For
example, some hardware combinations would report a locally initiated
disconnection even if the user turned Bluetooth off in the remote side.

Signed-off-by: Mikel Astiz <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 +-
include/net/bluetooth/mgmt.h | 9 +++++++++
net/bluetooth/hci_event.c | 26 +++++++++++++++++++++++---
net/bluetooth/mgmt.c | 9 +++++----
4 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 41d9439..4fb0323 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1004,7 +1004,7 @@ int mgmt_device_connected(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
u8 addr_type, u32 flags, u8 *name, u8 name_len,
u8 *dev_class);
int mgmt_device_disconnected(struct hci_dev *hdev, bdaddr_t *bdaddr,
- u8 link_type, u8 addr_type);
+ u8 link_type, u8 addr_type, u8 reason);
int mgmt_disconnect_failed(struct hci_dev *hdev, bdaddr_t *bdaddr,
u8 link_type, u8 addr_type, u8 status);
int mgmt_connect_failed(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 4348ee8..1b48eff 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -405,7 +405,16 @@ struct mgmt_ev_device_connected {
__u8 eir[0];
} __packed;

+#define MGMT_DEV_DISCONN_UNKNOWN 0x00
+#define MGMT_DEV_DISCONN_TIMEOUT 0x01
+#define MGMT_DEV_DISCONN_LOCAL_HOST 0x02
+#define MGMT_DEV_DISCONN_REMOTE 0x03
+
#define MGMT_EV_DEVICE_DISCONNECTED 0x000C
+struct mgmt_ev_device_disconnected {
+ struct mgmt_addr_info addr;
+ __u8 reason;
+} __packed;

#define MGMT_EV_CONNECT_FAILED 0x000D
struct mgmt_ev_connect_failed {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 32fcc74..6e02c93 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -29,6 +29,7 @@

#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
+#include <net/bluetooth/mgmt.h>

/* Handle HCI Event packets */

@@ -1888,6 +1889,22 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
}
}

+static u8 hci_to_mgmt_reason(u8 err)
+{
+ switch (err) {
+ case HCI_ERROR_CONNECTION_TIMEOUT:
+ return MGMT_DEV_DISCONN_TIMEOUT;
+ case HCI_ERROR_REMOTE_USER_TERM:
+ case HCI_ERROR_REMOTE_LOW_RESOURCES:
+ case HCI_ERROR_REMOTE_POWER_OFF:
+ return MGMT_DEV_DISCONN_REMOTE;
+ case HCI_ERROR_LOCAL_HOST_TERM:
+ return MGMT_DEV_DISCONN_LOCAL_HOST;
+ default:
+ return MGMT_DEV_DISCONN_UNKNOWN;
+ }
+}
+
static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_disconn_complete *ev = (void *) skb->data;
@@ -1906,12 +1923,15 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)

if (test_and_clear_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags) &&
(conn->type == ACL_LINK || conn->type == LE_LINK)) {
- if (ev->status)
+ if (ev->status) {
mgmt_disconnect_failed(hdev, &conn->dst, conn->type,
conn->dst_type, ev->status);
- else
+ } else {
+ u8 reason = hci_to_mgmt_reason(ev->reason);
+
mgmt_device_disconnected(hdev, &conn->dst, conn->type,
- conn->dst_type);
+ conn->dst_type, reason);
+ }
}

if (ev->status == 0) {
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a3329cb..05d4b83 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3077,16 +3077,17 @@ static void unpair_device_rsp(struct pending_cmd *cmd, void *data)
}

int mgmt_device_disconnected(struct hci_dev *hdev, bdaddr_t *bdaddr,
- u8 link_type, u8 addr_type)
+ u8 link_type, u8 addr_type, u8 reason)
{
- struct mgmt_addr_info ev;
+ struct mgmt_ev_device_disconnected ev;
struct sock *sk = NULL;
int err;

mgmt_pending_foreach(MGMT_OP_DISCONNECT, hdev, disconnect_rsp, &sk);

- bacpy(&ev.bdaddr, bdaddr);
- ev.type = link_to_bdaddr(link_type, addr_type);
+ bacpy(&ev.addr.bdaddr, bdaddr);
+ ev.addr.type = link_to_bdaddr(link_type, addr_type);
+ ev.reason = reason;

err = mgmt_event(MGMT_EV_DEVICE_DISCONNECTED, hdev, &ev, sizeof(ev),
sk);
--
1.7.7.6