* [RFC v3 0/2] Disconnect reason in Kernel API
@ 2012-08-08 7:42 Mikel Astiz
2012-08-08 7:42 ` [RFC v3 1/2] Bluetooth: Add more HCI error codes Mikel Astiz
2012-08-08 7:42 ` [RFC v3 2/2] Bluetooth: mgmt: Add device disconnect reason Mikel Astiz
0 siblings, 2 replies; 6+ messages in thread
From: Mikel Astiz @ 2012-08-08 7:42 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
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
^ permalink raw reply [flat|nested] 6+ messages in thread* [RFC v3 1/2] Bluetooth: Add more HCI error codes 2012-08-08 7:42 [RFC v3 0/2] Disconnect reason in Kernel API Mikel Astiz @ 2012-08-08 7:42 ` Mikel Astiz 2012-08-08 13:48 ` Marcel Holtmann 2012-08-08 7:42 ` [RFC v3 2/2] Bluetooth: mgmt: Add device disconnect reason Mikel Astiz 1 sibling, 1 reply; 6+ messages in thread From: Mikel Astiz @ 2012-08-08 7:42 UTC (permalink / raw) To: linux-bluetooth; +Cc: Mikel Astiz From: Mikel Astiz <mikel.astiz@bmw-carit.de> Add more HCI error codes as defined in the specification. Signed-off-by: Mikel Astiz <mikel.astiz@bmw-carit.de> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC v3 1/2] Bluetooth: Add more HCI error codes 2012-08-08 7:42 ` [RFC v3 1/2] Bluetooth: Add more HCI error codes Mikel Astiz @ 2012-08-08 13:48 ` Marcel Holtmann 0 siblings, 0 replies; 6+ messages in thread From: Marcel Holtmann @ 2012-08-08 13:48 UTC (permalink / raw) To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz Hi Mikel, > Add more HCI error codes as defined in the specification. > > Signed-off-by: Mikel Astiz <mikel.astiz@bmw-carit.de> > --- > include/net/bluetooth/hci.h | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) Acked-by: Marcel Holtmann <marcel@holtmann.org> Regards Marcel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC v3 2/2] Bluetooth: mgmt: Add device disconnect reason 2012-08-08 7:42 [RFC v3 0/2] Disconnect reason in Kernel API Mikel Astiz 2012-08-08 7:42 ` [RFC v3 1/2] Bluetooth: Add more HCI error codes Mikel Astiz @ 2012-08-08 7:42 ` Mikel Astiz 2012-08-08 13:52 ` Marcel Holtmann 1 sibling, 1 reply; 6+ messages in thread From: Mikel Astiz @ 2012-08-08 7:42 UTC (permalink / raw) To: linux-bluetooth; +Cc: Mikel Astiz From: Mikel Astiz <mikel.astiz@bmw-carit.de> 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 <mikel.astiz@bmw-carit.de> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC v3 2/2] Bluetooth: mgmt: Add device disconnect reason 2012-08-08 7:42 ` [RFC v3 2/2] Bluetooth: mgmt: Add device disconnect reason Mikel Astiz @ 2012-08-08 13:52 ` Marcel Holtmann 2012-08-09 7:43 ` Mikel Astiz 0 siblings, 1 reply; 6+ messages in thread From: Marcel Holtmann @ 2012-08-08 13:52 UTC (permalink / raw) To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz 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 <mikel.astiz@bmw-carit.de> > --- > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC v3 2/2] Bluetooth: mgmt: Add device disconnect reason 2012-08-08 13:52 ` Marcel Holtmann @ 2012-08-09 7:43 ` Mikel Astiz 0 siblings, 0 replies; 6+ messages in thread From: Mikel Astiz @ 2012-08-09 7:43 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth, Mikel Astiz Hi Marcel, On Wed, Aug 8, 2012 at 3:52 PM, Marcel Holtmann <marcel@holtmann.org> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-08-09 7:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-08 7:42 [RFC v3 0/2] Disconnect reason in Kernel API Mikel Astiz 2012-08-08 7:42 ` [RFC v3 1/2] Bluetooth: Add more HCI error codes Mikel Astiz 2012-08-08 13:48 ` Marcel Holtmann 2012-08-08 7:42 ` [RFC v3 2/2] Bluetooth: mgmt: Add device disconnect reason Mikel Astiz 2012-08-08 13:52 ` Marcel Holtmann 2012-08-09 7:43 ` Mikel Astiz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox