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@xxxxxxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
