From: Mikel Astiz <[email protected]>
This patchset (split now in two patches) extends the management API to expose the disconnect reason in the device disconnection event.
The original motivation was to distinguish the timeout case from the rest, thus the first versions of the patch provided only this information.
v3 extends the original approach to also determine which side (local host or remote) initiated the ACL disconnection. This is IMO not very useful and misleading, but has still been considered interesting by the subsystem maintainers.
In order to test these patches, the old userland patch ("[RFC BlueZ v1]: mgmt-api: Add reason to device disconnect event") can be used, in terms of log traces, even though it needs the corresponding update.
>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 (2):
Bluetooth: Add more HCI error codes
Bluetooth: mgmt: Add device disconnect reason
include/net/bluetooth/hci.h | 3 +++
include/net/bluetooth/hci_core.h | 2 +-
include/net/bluetooth/mgmt.h | 8 ++++++++
net/bluetooth/hci_event.c | 24 +++++++++++++++++++++---
net/bluetooth/mgmt.c | 9 +++++----
5 files changed, 38 insertions(+), 8 deletions(-)
--
1.7.7.6
Hi Marcel,
On Wed, Aug 8, 2012 at 3:52 PM, Marcel Holtmann <[email protected]> wrote:
> 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 ACL connection timeout
>> 0x02 ACL connection terminated by local host
>> 0x03 ACL connection terminated by remote host
>
> I think we need to leave ACL out here. Since that is not how we defined
> what a connection is in mgmt terms.
>
>> Note that the local/remote distinction just determines which side
>> terminated the low-level ACL 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.
>
> Please make sure that this comment makes it also into the API
> description.
I will send an updated version soon, with all your suggestions, along
with the updated userland patch where you can see the API
documentation.
<snip>
>> @@ -1906,12 +1907,29 @@ 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 != 0) {
>
> While at it, turn this into if (ev->status).
I will send a separate patch for this and other similar occurrences.
Let me know if you prefer to squash the change.
Cheers,
Mikel
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 ACL connection timeout
> 0x02 ACL connection terminated by local host
> 0x03 ACL connection terminated by remote host
I think we need to leave ACL out here. Since that is not how we defined
what a connection is in mgmt terms.
> Note that the local/remote distinction just determines which side
> terminated the low-level ACL 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.
Please make sure that this comment makes it also into the API
description.
>
> Signed-off-by: Mikel Astiz <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 2 +-
> include/net/bluetooth/mgmt.h | 8 ++++++++
> net/bluetooth/hci_event.c | 24 +++++++++++++++++++++---
> net/bluetooth/mgmt.c | 9 +++++----
> 4 files changed, 35 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..e8b86e0 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -405,7 +405,15 @@ struct mgmt_ev_device_connected {
> __u8 eir[0];
> } __packed;
>
Please also list MGMT_DEV_DISCONN_UNKNOWN here.
> +#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 0386e1e..1323341 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 */
>
> @@ -1906,12 +1907,29 @@ 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 != 0) {
While at it, turn this into if (ev->status).
> mgmt_disconnect_failed(hdev, &conn->dst, conn->type,
> conn->dst_type, ev->status);
> - else
> + } else {
> + u8 reason = 0;
> +
> + switch (ev->reason) {
> + case HCI_ERROR_CONNECTION_TIMEOUT:
> + reason = MGMT_DEV_DISCONN_TIMEOUT;
> + break;
> + case HCI_ERROR_REMOTE_USER_TERM:
> + case HCI_ERROR_REMOTE_LOW_RESOURCES:
> + case HCI_ERROR_REMOTE_POWER_OFF:
> + reason = MGMT_DEV_DISCONN_REMOTE;
> + break;
> + case HCI_ERROR_LOCAL_HOST_TERM:
> + reason = MGMT_DEV_DISCONN_LOCAL_HOST;
> + break;
> + }
I would prefer a helper function to turn HCI error into 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);
Otherwise I am fine with this.
Regards
Marcel
Hi Mikel,
> Add more HCI error codes as defined in the specification.
>
> Signed-off-by: Mikel Astiz <[email protected]>
> ---
> include/net/bluetooth/hci.h | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
Acked-by: Marcel Holtmann <[email protected]>
Regards
Marcel
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 ACL connection timeout
0x02 ACL connection terminated by local host
0x03 ACL connection terminated by remote host
Note that the local/remote distinction just determines which side
terminated the low-level ACL 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 | 8 ++++++++
net/bluetooth/hci_event.c | 24 +++++++++++++++++++++---
net/bluetooth/mgmt.c | 9 +++++----
4 files changed, 35 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..e8b86e0 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -405,7 +405,15 @@ struct mgmt_ev_device_connected {
__u8 eir[0];
} __packed;
+#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 0386e1e..1323341 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 */
@@ -1906,12 +1907,29 @@ 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 != 0) {
mgmt_disconnect_failed(hdev, &conn->dst, conn->type,
conn->dst_type, ev->status);
- else
+ } else {
+ u8 reason = 0;
+
+ switch (ev->reason) {
+ case HCI_ERROR_CONNECTION_TIMEOUT:
+ reason = MGMT_DEV_DISCONN_TIMEOUT;
+ break;
+ case HCI_ERROR_REMOTE_USER_TERM:
+ case HCI_ERROR_REMOTE_LOW_RESOURCES:
+ case HCI_ERROR_REMOTE_POWER_OFF:
+ reason = MGMT_DEV_DISCONN_REMOTE;
+ break;
+ case HCI_ERROR_LOCAL_HOST_TERM:
+ reason = MGMT_DEV_DISCONN_LOCAL_HOST;
+ break;
+ }
+
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
From: Mikel Astiz <[email protected]>
Add more HCI error codes as defined in the specification.
Signed-off-by: Mikel Astiz <[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