* Re: [PATCH 1/2 v2] Bluetooth: Fix system crash caused by del_timer()
From: Gustavo F. Padovan @ 2010-10-22 17:18 UTC (permalink / raw)
To: Haijun Liu; +Cc: linux-bluetooth
In-Reply-To: <1287714419-13545-1-git-send-email-haijun.liu@atheros.com>
* Haijun Liu <haijun.liu@atheros.com> [2010-10-22 10:26:58 +0800]:
> During test session with another vendor's bt stack, found that in
> l2cap_chan_del() using del_timer() caused l2cap_monitor_timeout()
> be called after the sock was freed, so it raised a system crash.
> So I just replaced del_timer() with del_timer_sync() to solve it.
NAK on this. If you read the del_timer_sync() documentation you can
see that you can't call del_timer_sync() on interrupt context. The
possible solution here is to check in the beginning of
l2cap_monitor_timeout() if your sock is still valid.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 2/2 v2] Bluetooth: Fix system crash bug of no send queue protect
From: Gustavo F. Padovan @ 2010-10-22 17:34 UTC (permalink / raw)
To: Haijun Liu; +Cc: linux-bluetooth
In-Reply-To: <1287714419-13545-2-git-send-email-haijun.liu@atheros.com>
* Haijun Liu <haijun.liu@atheros.com> [2010-10-22 10:26:59 +0800]:
> During test session with another vendor's bt stack, found that
> without lock protect for TX_QUEUE(sk) will cause system crash while
> data transfer over AMP controller. So I just add lock protect for
> TX_QUEUE(sk).
We already use the default socket lock protection. Is it not working for
you? Why? Could you show a crash case that requires your patch to fix
it?
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 2/6] Bluetooth: Add LE connect support
From: Gustavo F. Padovan @ 2010-10-22 18:46 UTC (permalink / raw)
To: Ville Tervo; +Cc: linux-bluetooth
In-Reply-To: <1287406976-13463-3-git-send-email-ville.tervo@nokia.com>
* Ville Tervo <ville.tervo@nokia.com> [2010-10-18 16:02:52 +0300]:
> Bluetooth V4.0 adds support for Low Energy (LE)
> connections. Specification introduses new set
> of hci commands to control LE connection.
> This patch adds logic to create, cancel and
> disconnect LE connections
>
> Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> ---
> include/net/bluetooth/hci.h | 2 +
> include/net/bluetooth/hci_core.h | 21 +++++++--
> net/bluetooth/hci_conn.c | 51 +++++++++++++++++++-
> net/bluetooth/hci_event.c | 94 +++++++++++++++++++++++++++++++++++++-
> 4 files changed, 160 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index ee5beec..02055b9 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -159,6 +159,8 @@ enum {
> #define SCO_LINK 0x00
> #define ACL_LINK 0x01
> #define ESCO_LINK 0x02
> +/* Low Energy links do not have defined link type. Use invented one */
> +#define LE_LINK 0x80
>
> /* LMP features */
> #define LMP_3SLOT 0x01
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ebec8c9..fc2aaee 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -60,6 +60,7 @@ struct hci_conn_hash {
> spinlock_t lock;
> unsigned int acl_num;
> unsigned int sco_num;
> + unsigned int le_num;
> };
>
> struct bdaddr_list {
> @@ -272,20 +273,32 @@ static inline void hci_conn_hash_add(struct hci_dev *hdev, struct hci_conn *c)
> {
> struct hci_conn_hash *h = &hdev->conn_hash;
> list_add(&c->list, &h->list);
> - if (c->type == ACL_LINK)
> + switch (c->type) {
> + case ACL_LINK:
> h->acl_num++;
> - else
> + break;
> + case LE_LINK:
> + h->le_num++;
> + break;
> + default:
I would add a 'case SCO_LINK' here. It changes nothing actually, but
make switch easy to understand.
> h->sco_num++;
> + }
> }
>
> static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c)
> {
> struct hci_conn_hash *h = &hdev->conn_hash;
> list_del(&c->list);
> - if (c->type == ACL_LINK)
> + switch (c->type) {
> + case ACL_LINK:
> h->acl_num--;
> - else
> + break;
> + case LE_LINK:
> + h->le_num--;
> + break;
> + default:
Same here.
> h->sco_num--;
> + }
> }
>
> static inline struct hci_conn *hci_conn_hash_lookup_handle(struct hci_dev *hdev,
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 0b1e460..c1eb8e0 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -45,6 +45,32 @@
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
>
> +void hci_le_connect(struct hci_conn *conn)
> +{
> + struct hci_dev *hdev = conn->hdev;
> + struct hci_cp_le_create_conn cp;
> +
> + conn->state = BT_CONNECT;
> + conn->out = 1;
> +
> + memset(&cp, 0, sizeof(cp));
> + cp.scan_interval = cpu_to_le16(0x0004);
> + cp.scan_window = cpu_to_le16(0x0004);
> + bacpy(&cp.peer_addr, &conn->dst);
> + cp.conn_interval_min = cpu_to_le16(0x0008);
> + cp.conn_interval_max = cpu_to_le16(0x0100);
> + cp.supervision_timeout = cpu_to_le16(0x0064);
> + cp.min_ce_len = cpu_to_le16(0x0001);
> + cp.max_ce_len = cpu_to_le16(0x0001);
> +
> + hci_send_cmd(hdev, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
> +}
> +
> +static void hci_le_connect_cancel(struct hci_conn *conn)
> +{
> + hci_send_cmd(conn->hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
> +}
> +
> void hci_acl_connect(struct hci_conn *conn)
> {
> struct hci_dev *hdev = conn->hdev;
> @@ -192,8 +218,12 @@ static void hci_conn_timeout(unsigned long arg)
> switch (conn->state) {
> case BT_CONNECT:
> case BT_CONNECT2:
> - if (conn->type == ACL_LINK && conn->out)
> - hci_acl_connect_cancel(conn);
> + if (conn->out) {
> + if (conn->type == ACL_LINK)
> + hci_acl_connect_cancel(conn);
> + else if (conn->type == LE_LINK)
> + hci_le_connect_cancel(conn);
> + }
> break;
> case BT_CONFIG:
> case BT_CONNECTED:
> @@ -359,15 +389,30 @@ struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src)
> }
> EXPORT_SYMBOL(hci_get_route);
>
> -/* Create SCO or ACL connection.
> +/* Create SCO, ACL or LE connection.
> * Device _must_ be locked */
> struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8 sec_level, __u8 auth_type)
> {
> struct hci_conn *acl;
> struct hci_conn *sco;
> + struct hci_conn *le;
>
> BT_DBG("%s dst %s", hdev->name, batostr(dst));
>
> + if (type == LE_LINK) {
> + le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
> +
> + if (!le)
> + le = hci_conn_add(hdev, LE_LINK, dst);
> +
> + if (!le)
> + return NULL;
> +
> + hci_le_connect(le);
> +
> + return le;
> + }
> +
> if (!(acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst))) {
> if (!(acl = hci_conn_add(hdev, ACL_LINK, dst)))
> return NULL;
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 84093b0..4061613 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -822,6 +822,43 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
> hci_dev_unlock(hdev);
> }
>
> +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%x", hdev->name, status);
> +
> + cp = hci_sent_cmd_data(hdev, HCI_OP_LE_CREATE_CONN);
> + if (!cp)
> + return;
> +
> + hci_dev_lock(hdev);
> +
> + conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->peer_addr);
> +
> + 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;
> + hci_proto_connect_cfm(conn, status);
> + hci_conn_del(conn);
> + }
> + } else {
> + if (!conn) {
> + conn = hci_conn_add(hdev, LE_LINK, &cp->peer_addr);
> + if (conAvoid things like that in your patch.n)
> + conn->out = 1;
> + else
> + BT_ERR("No memory for new connection");
> + }
> + }
> +
> + hci_dev_unlock(hdev);
> +}
> +
> static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> {
> __u8 status = *((__u8 *) skb->data);
> @@ -1024,7 +1061,6 @@ static inline void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff
> conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(ev->handle));
> if (conn) {
> conn->state = BT_CLOSED;
> -
Avoid things like that in your patch.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 3/6] Bluetooth: Use LE buffers for LE traffic
From: Gustavo F. Padovan @ 2010-10-22 18:53 UTC (permalink / raw)
To: Ville Tervo; +Cc: linux-bluetooth
In-Reply-To: <1287406976-13463-4-git-send-email-ville.tervo@nokia.com>
Hi Ville,
* Ville Tervo <ville.tervo@nokia.com> [2010-10-18 16:02:53 +0300]:
> BLuetooth chips may have separate buffers for
> LE traffic. This patch add support to use LE
> buffers provided by the chip.
>
> Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> ---
> include/net/bluetooth/hci.h | 2 +
> include/net/bluetooth/hci_core.h | 5 +++
> net/bluetooth/hci_conn.c | 6 +++
> net/bluetooth/hci_core.c | 74 +++++++++++++++++++++++++++++++++++--
> net/bluetooth/hci_event.c | 40 +++++++++++++++++++-
> 5 files changed, 121 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 02055b9..b42edf0 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -189,6 +189,8 @@ enum {
>
> #define LMP_EV4 0x01
> #define LMP_EV5 0x02
> +#define LMP_NO_BREDR 0x20
You are not using this one.
> +#define LMP_LE 0x40
>
> #define LMP_SNIFF_SUBR 0x02
> #define LMP_EDR_ESCO_2M 0x20
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index fc2aaee..326d290 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -103,15 +103,19 @@ struct hci_dev {
> atomic_t cmd_cnt;
> unsigned int acl_cnt;
> unsigned int sco_cnt;
> + unsigned int le_cnt;
>
> unsigned int acl_mtu;
> unsigned int sco_mtu;
> + unsigned int le_mtu;
> unsigned int acl_pkts;
> unsigned int sco_pkts;
> + unsigned int le_pkts;
>
> unsigned long cmd_last_tx;
> unsigned long acl_last_tx;
> unsigned long sco_last_tx;
> + unsigned long le_last_tx;
>
> struct workqueue_struct *workqueue;
>
> @@ -469,6 +473,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
> #define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
> #define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
> #define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
> +#define lmp_le_capable(dev) ((dev)->features[4] & LMP_LE)
>
> /* ----- HCI protocols ----- */
> struct hci_proto {
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index c1eb8e0..c7309e4 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -324,6 +324,11 @@ int hci_conn_del(struct hci_conn *conn)
>
> /* Unacked frames */
> hdev->acl_cnt += conn->sent;
> + } else if (conn->type == LE_LINK) {
> + if (hdev->le_pkts)
> + hdev->le_cnt += conn->sent;
> + else
> + hdev->acl_cnt += conn->sent;
> } else {
> struct hci_conn *acl = conn->link;
> if (acl) {
> @@ -409,6 +414,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
> return NULL;
>
> hci_le_connect(le);
> + hci_conn_hold(le);
>
This should be in 2/6, right?
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 4/6] Bluetooth: Add LE connection support to L2CAP
From: Gustavo F. Padovan @ 2010-10-22 19:14 UTC (permalink / raw)
To: Ville Tervo; +Cc: linux-bluetooth
In-Reply-To: <1287406976-13463-5-git-send-email-ville.tervo@nokia.com>
Hi Ville,
* Ville Tervo <ville.tervo@nokia.com> [2010-10-18 16:02:54 +0300]:
> Add basic LE connection support to L2CAP. LE
> connection can be created by specifying cid
> in struct sockaddr_l2
>
> Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
> ---
> include/net/bluetooth/l2cap.h | 3 +++
> net/bluetooth/l2cap.c | 32 ++++++++++++++++++++++++--------
> 2 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index c819c8b..cc3a140 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -160,6 +160,9 @@ struct l2cap_conn_rsp {
> /* channel indentifier */
> #define L2CAP_CID_SIGNALING 0x0001
> #define L2CAP_CID_CONN_LESS 0x0002
> +#define L2CAP_CID_LE_DATA 0x0004
> +#define L2CAP_CID_LE_SIGNALING 0x0005
> +#define L2CAP_CID_SMP 0x0006
> #define L2CAP_CID_DYN_START 0x0040
> #define L2CAP_CID_DYN_END 0xffff
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 16049de..bf5daf3 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -617,6 +617,12 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
> for (sk = l->head; sk; sk = l2cap_pi(sk)->next_c) {
> bh_lock_sock(sk);
>
> + if (conn->hcon->type == LE_LINK) {
> + l2cap_sock_clear_timer(sk);
> + sk->sk_state = BT_CONNECTED;
> + sk->sk_state_change(sk);
> + }
> +
> if (sk->sk_type != SOCK_SEQPACKET &&
> sk->sk_type != SOCK_STREAM) {
> l2cap_sock_clear_timer(sk);
> @@ -675,7 +681,11 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status)
>
> BT_DBG("hcon %p conn %p", hcon, conn);
>
> - conn->mtu = hcon->hdev->acl_mtu;
> + if (hcon->hdev->le_mtu && hcon->type == LE_LINK)
> + conn->mtu = hcon->hdev->le_mtu;
> + else
> + conn->mtu = hcon->hdev->acl_mtu;
> +
> conn->src = &hcon->hdev->bdaddr;
> conn->dst = &hcon->dst;
>
> @@ -1102,8 +1112,13 @@ static int l2cap_do_connect(struct sock *sk)
> }
> }
>
> - hcon = hci_connect(hdev, ACL_LINK, dst,
> + if (l2cap_pi(sk)->dcid == L2CAP_CID_LE_DATA)
> + hcon = hci_connect(hdev, LE_LINK, dst,
> + l2cap_pi(sk)->sec_level, auth_type);
> + else
I think that "else if (l2cap_pi(sk)->psm)" is better here, we do not
want to permit go ahead with psm 0.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* [PATCH 1/1] Fix small coding style issues
From: Elvis Pfützenreuter @ 2010-10-22 19:18 UTC (permalink / raw)
To: linux-bluetooth; +Cc: epx
Based on http://lxr.linux.no/linux+v2.6.36/Documentation/CodingStyle#L171
---
health/hdp_util.c | 24 +++++++++++++-----------
health/mcap_lib.h | 8 ++++----
2 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/health/hdp_util.c b/health/hdp_util.c
index b0399f8..fefb6d3 100644
--- a/health/hdp_util.c
+++ b/health/hdp_util.c
@@ -181,8 +181,9 @@ static gboolean parse_role(DBusMessageIter *iter, gpointer data, GError **err)
dbus_message_iter_recurse(iter, &value);
ctype = dbus_message_iter_get_arg_type(&value);
string = &value;
- } else
+ } else {
string = iter;
+ }
if (ctype != DBUS_TYPE_STRING) {
g_set_error(err, HDP_ERROR, HDP_UNSPECIFIED_ERROR,
@@ -191,11 +192,11 @@ static gboolean parse_role(DBusMessageIter *iter, gpointer data, GError **err)
}
dbus_message_iter_get_basic(string, &role);
- if (g_ascii_strcasecmp(role, HDP_SINK_ROLE_AS_STRING) == 0)
+ if (g_ascii_strcasecmp(role, HDP_SINK_ROLE_AS_STRING) == 0) {
app->role = HDP_SINK;
- else if (g_ascii_strcasecmp(role, HDP_SOURCE_ROLE_AS_STRING) == 0)
+ } else if (g_ascii_strcasecmp(role, HDP_SOURCE_ROLE_AS_STRING) == 0) {
app->role = HDP_SOURCE;
- else {
+ } else {
g_set_error(err, HDP_ERROR, HDP_UNSPECIFIED_ERROR,
"Role value should be \"source\" or \"sink\"");
return FALSE;
@@ -221,8 +222,9 @@ static gboolean parse_desc(DBusMessageIter *iter, gpointer data, GError **err)
dbus_message_iter_recurse(iter, &variant);
ctype = dbus_message_iter_get_arg_type(&variant);
string = &variant;
- } else
+ } else {
string = iter;
+ }
if (ctype != DBUS_TYPE_STRING) {
g_set_error(err, HDP_ERROR, HDP_DIC_ENTRY_PARSE_ERROR,
@@ -395,12 +397,12 @@ static gboolean register_service_protocols(struct hdp_adapter *adapter,
goto end;
}
- if (!sdp_list_append( mcap_list, mcap_ver)) {
+ if (!sdp_list_append(mcap_list, mcap_ver)) {
ret = FALSE;
goto end;
}
- if (!sdp_list_append( proto_list, mcap_list)) {
+ if (!sdp_list_append(proto_list, mcap_list)) {
ret = FALSE;
goto end;
}
@@ -442,7 +444,7 @@ static gboolean register_service_profiles(sdp_record_t *sdp_record)
sdp_profile_desc_t hdp_profile;
/* set hdp information */
- sdp_uuid16_create( &hdp_profile.uuid, HDP_SVCLASS_ID);
+ sdp_uuid16_create(&hdp_profile.uuid, HDP_SVCLASS_ID);
hdp_profile.version = HDP_VERSION;
profile_list = sdp_list_append(NULL, &hdp_profile);
if (!profile_list)
@@ -459,7 +461,7 @@ static gboolean register_service_profiles(sdp_record_t *sdp_record)
return ret;
}
-static gboolean register_service_aditional_protocols(
+static gboolean register_service_additional_protocols(
struct hdp_adapter *adapter,
sdp_record_t *sdp_record)
{
@@ -502,7 +504,7 @@ static gboolean register_service_aditional_protocols(
goto end;
}
- if (!sdp_list_append( proto_list, mcap_list)) {
+ if (!sdp_list_append(proto_list, mcap_list)) {
ret = FALSE;
goto end;
}
@@ -709,7 +711,7 @@ gboolean hdp_update_sdp_record(struct hdp_adapter *adapter, GSList *app_list)
goto fail;
if (!register_service_profiles(sdp_record))
goto fail;
- if (!register_service_aditional_protocols(adapter, sdp_record))
+ if (!register_service_additional_protocols(adapter, sdp_record))
goto fail;
sdp_set_info_attr(sdp_record, HDP_SERVICE_NAME, HDP_SERVICE_PROVIDER,
diff --git a/health/mcap_lib.h b/health/mcap_lib.h
index daee1f8..7740623 100644
--- a/health/mcap_lib.h
+++ b/health/mcap_lib.h
@@ -69,7 +69,7 @@ struct sync_info_ind_data;
/************ Callbacks ************/
-/* mdl callbacks */
+/* MDL callbacks */
typedef void (* mcap_mdl_event_cb) (struct mcap_mdl *mdl, gpointer data);
typedef void (* mcap_mdl_operation_conf_cb) (struct mcap_mdl *mdl, uint8_t conf,
@@ -85,7 +85,7 @@ typedef uint8_t (* mcap_remote_mdl_conn_req_cb) (struct mcap_mcl *mcl,
typedef uint8_t (* mcap_remote_mdl_reconn_req_cb) (struct mcap_mdl *mdl,
gpointer data);
-/* mcl callbacks */
+/* MCL callbacks */
typedef void (* mcap_mcl_event_cb) (struct mcap_mcl *mcl, gpointer data);
typedef void (* mcap_mcl_connect_cb) (struct mcap_mcl *mcl, GError *err,
@@ -115,7 +115,7 @@ typedef void (* mcap_sync_set_cb) (struct mcap_mcl *mcl,
/************ Operations ************/
-/* Mdl operations*/
+/* MDL operations */
gboolean mcap_create_mdl(struct mcap_mcl *mcl,
uint8_t mdepid,
@@ -155,7 +155,7 @@ gboolean mcap_mdl_abort(struct mcap_mdl *mdl,
int mcap_mdl_get_fd(struct mcap_mdl *mdl);
uint16_t mcap_mdl_get_mdlid(struct mcap_mdl *mdl);
-/* Mcl operations*/
+/* MCL operations */
gboolean mcap_create_mcl(struct mcap_instance *ms,
const bdaddr_t *addr,
--
1.7.0.4
^ permalink raw reply related
* (Health) ChannelConnected signal when MDL aborted?
From: Elvis Pfützenreuter @ 2010-10-22 20:10 UTC (permalink / raw)
To: Santiago Carot-Nemesio, Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
Taking a look in the code, I saw that hdp_mcap_mdl_aborted_cb() emits a ChannelConnected signal.
I understand that after an abort, the MDL still exists, and can be reconnected, so I understand the intention of informing the application that a channel has been "created". But I'm not sure that it is opportune.
The first thing the application will do is trying to Acquire() the channel's file descriptor (that does not exist) and that triggers a reconnection attempt, from acceptor to initiator. While the most sensible thing (in my view) is waiting for the initiator to reconnect -- if it aborted, it must have a good reason, and it will probably reject the immediate call back.
What do you think?
^ permalink raw reply
* Re: [PATCH] Fix crash when GetProperties req is received before any adapters are set up
From: Luiz Augusto von Dentz @ 2010-10-22 20:25 UTC (permalink / raw)
To: ext-tommi.keisala, linux-bluetooth
In-Reply-To: <20101022131207.GA29655@jh-x301>
Hi,
On Fri, Oct 22, 2010 at 4:12 PM, Johan Hedberg <johan.hedberg@nokia.com> wrote:
> Hi Tommi,
>
> On Fri, Oct 22, 2010, ext-tommi.keisala@nokia.com wrote:
>> This patch avoids a crash when org.bluez.Manager GetProperties request is
>> received and there is not yet any adapters ready. Happens often for example
>> when bluetoothd and ofonod is started next ot each other.
>>
>> Signed-off-by: Tommi Keisala <ext-tommi.keisala@nokia.com>
>> ---
>> src/manager.c | 6 +++++-
>> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> Looks like a bugfix is definitely in order here, but why introduce a new
> variable to the function? Wouldn't something like the folling be good
> enough:
>
> --- a/src/manager.c
> +++ b/src/manager.c
> @@ -197,13 +197,14 @@ static DBusMessage *get_properties(DBusConnection *conn,
> DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
>
> array = g_new0(char *, g_slist_length(adapters) + 1);
> - for (i = 0, list = adapters; list; list = list->next, i++) {
> + for (i = 0, list = adapters; list; list = list->next) {
> struct btd_adapter *adapter = list->data;
>
> if (!adapter_is_ready(adapter))
> continue;
>
> array[i] = (char *) adapter_get_path(adapter);
> + i++;
> }
> dict_append_array(&dict, "Adapters", DBUS_TYPE_OBJECT_PATH, &array, i);
> g_free(array);
>
> Could you send a new patch which doesn't introduce a new variable? Also,
> please leave out the signed-off-by from the commit message since it's
> not used for userspace bluez patches. Thanks.
Maybe we should also add a check here to omit adapters if we don't
have at least one to append, it should be fine to have empty
containers but I don't think this is very useful for the application
since they have to iterate in the container to find out there is in
fact nothing there.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH] Fix crash when GetProperties req is received before any adapters are set up
From: Johan Hedberg @ 2010-10-22 20:40 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: ext-tommi.keisala, linux-bluetooth
In-Reply-To: <AANLkTimwz0wYbiQpkPmJuNpmJxNxaAQ2UtudPqinh9kP@mail.gmail.com>
Hi Luiz,
On Fri, Oct 22, 2010, Luiz Augusto von Dentz wrote:
> Maybe we should also add a check here to omit adapters if we don't
> have at least one to append, it should be fine to have empty
> containers but I don't think this is very useful for the application
> since they have to iterate in the container to find out there is in
> fact nothing there.
Wouldn't it complicate e.g. python code in that you'd need
if manager_props.has_key('Adapters'):
for adapter in manager_props['Adapters']:
...
to avoid triggering a KeyError exception which could happen if you try
to access manager_props['Adapters'] directly.
Johan
^ permalink raw reply
* [RFC] SMP initial draft
From: Anderson Briglia @ 2010-10-22 23:56 UTC (permalink / raw)
To: linux-bluetooth
This RFC is about Security Manager Protocol (SMP).
Basically this patchset implements the initial negotiation over L2CAP and LE
connections between a Master and a Slave. TK and STK keys are not being generated
and/or exchanged yet, do not expect real encryption/decryption by now.
Actually, our next tasks are related to integrate current implementation and
Criptographic Toolbox ah, c1 and s1 functions for TK and STK key generation for
Just Works SMP pairing method.
To test this RFC you need to have the Ville Tervo[1] kernel tree with LE connection
patches. Of course you need LE devices and bluez git tree (master branch). Just run
the bluetoothd and try to list the primary services using gatttool over an LE channel.
eg.: gatttool --primary --le -i hci0 -b 00:17:E7:DD:08:FF
Comments are welcome.
[1] git://git.kernel.org/pub/scm/linux/kernel/git/vtervo/bluetooth-le-2.6.git
[PATCH 1/6] Bluetooth: Add SMP command structures
[PATCH 2/6] Bluetooth: fix receiving L2CAP packets over LE
[PATCH 3/6] Bluetooth: Implement the first SMP commands
[PATCH 4/6] Bluetooth: Start SMP procedure
[PATCH 5/6] Bluetooth: Fix initiated LE connections
[PATCH 6/6] Bluetooth: simple SMP pairing negotiation
^ permalink raw reply
* [PATCH 1/6] Bluetooth: Add SMP command structures
From: Anderson Briglia @ 2010-10-22 23:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ville Tervo
From: Ville Tervo <ville.tervo@nokia.com>
Add command structures for security manager protocol.
Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
include/net/bluetooth/smp.h | 76 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 76 insertions(+), 0 deletions(-)
create mode 100644 include/net/bluetooth/smp.h
diff --git a/include/net/bluetooth/smp.h b/include/net/bluetooth/smp.h
new file mode 100644
index 0000000..8f2edbf
--- /dev/null
+++ b/include/net/bluetooth/smp.h
@@ -0,0 +1,76 @@
+#ifndef __SMP_H
+#define __SMP_H
+
+struct smp_command_hdr {
+ __u8 code;
+} __packed;
+
+#define SMP_CMD_PAIRING_REQ 0x01
+#define SMP_CMD_PAIRING_RSP 0x02
+struct smp_cmd_pairing {
+ __u8 io_capability;
+ __u8 oob_flag;
+ __u8 auth_req;
+ __u8 max_key_size;
+ __u8 init_key_dist;
+ __u8 resp_key_dist;
+} __packed;
+
+#define SMP_CMD_PAIRING_CONFIRM 0x03
+struct smp_cmd_pairing_confirm {
+ __u8 confirm_val[16];
+} __packed;
+
+#define SMP_CMD_PAIRING_RANDOM 0x04
+struct smp_cmd_pairing_random {
+ __u8 rand_val[16];
+} __packed;
+
+#define SMP_CMD_PAIRING_FAIL 0x05
+struct smp_cmd_pairing_fail {
+ __u8 reason;
+} __packed;
+
+#define SMP_CMD_ENCRYPT_INFO 0x06
+struct smp_cmd_encrypt_info {
+ __u8 ltk[16];
+} __packed;
+
+#define SMP_CMD_MASTER_IDENT 0x07
+struct smp_cmd_master_ident {
+ __u16 ediv;
+ __u8 rand[8];
+} __packed;
+
+#define SMP_CMD_IDENT_INFO 0x08
+struct smp_cmd_ident_info {
+ __u8 irk[16];
+} __packed;
+
+#define SMP_CMD_IDENT_ADDR_INFO 0x09
+struct smp_cmd_ident_addr_info {
+ __u8 addr_type;
+ bdaddr_t bdaddr;
+} __packed;
+
+#define SMP_CMD_SIGN_INFO 0x0a
+struct smp_cmd_sign_info {
+ __u8 csrk[16];
+} __packed;
+
+#define SMP_CMD_SECURITY_REQ 0x0b
+struct smp_cmd_security_req {
+ __u8 auth_req;
+} __packed;
+
+#define SMP_PASSKEY_ENTRY_FAILED 0x01
+#define SMP_OOB_NOT_AVAIL 0x02
+#define SMP_AUTH_REQUIREMENTS 0x03
+#define SMP_CONFIRM_FAILED 0x04
+#define SMP_PAIRING_NOTSUPP 0x05
+#define SMP_ENC_KEY_SIZE 0x06
+#define SMP_CMD_NOTSUPP 0x07
+#define SMP_UNSPECIFIED 0x08
+#define SMP_REPEATED_ATTEMPTS 0x09
+
+#endif /* __SMP_H */
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/6] Bluetooth: fix receiving L2CAP packets over LE
From: Anderson Briglia @ 2010-10-22 23:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
As L2CAP packets coming over LE don't have any more encapsulation,
other than L2CAP, we are able to process them as soon as they arrive.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/l2cap.c | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 2bf083e..1ac44f4 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4768,17 +4768,30 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
{
struct l2cap_conn *conn = hcon->l2cap_data;
+ struct l2cap_hdr *hdr;
+ int len;
if (!conn && !(conn = l2cap_conn_add(hcon, 0)))
goto drop;
BT_DBG("conn %p len %d flags 0x%x", conn, skb->len, flags);
+ if (hcon->type == LE_LINK) {
+ hdr = (struct l2cap_hdr *) skb->data;
+ len = __le16_to_cpu(hdr->len) + L2CAP_HDR_SIZE;
+
+ if (len == skb->len) {
+ /* Complete frame received */
+ l2cap_recv_frame(conn, skb);
+ return 0;
+ }
+
+ goto drop;
+ }
+
if (flags & ACL_START) {
- struct l2cap_hdr *hdr;
struct sock *sk;
u16 cid;
- int len;
if (conn->rx_len) {
BT_ERR("Unexpected start frame (len %d)", skb->len);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 3/6] Bluetooth: Implement the first SMP commands
From: Anderson Briglia @ 2010-10-22 23:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
These simple commands will allow the SMP procedure to be started
and terminated with a not supported error. This is the first step
toward something useful.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/l2cap.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 117 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 1ac44f4..ba87c84 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -54,6 +54,7 @@
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
#include <net/bluetooth/l2cap.h>
+#include <net/bluetooth/smp.h>
#define VERSION "2.15"
@@ -307,6 +308,85 @@ static void l2cap_chan_del(struct sock *sk, int err)
}
}
+static struct sk_buff *smp_build_cmd(struct l2cap_conn *conn, u8 code,
+ u16 dlen, void *data)
+{
+ struct sk_buff *skb;
+ struct l2cap_hdr *lh;
+ int len;
+
+ len = L2CAP_HDR_SIZE + 1 + dlen;
+
+ if (len > conn->mtu)
+ return NULL;
+
+ skb = bt_skb_alloc(len, GFP_ATOMIC);
+ if (!skb)
+ return NULL;
+
+ lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
+ lh->len = cpu_to_le16(1 + dlen);
+ lh->cid = cpu_to_le16(L2CAP_CID_SMP);
+
+ memcpy(skb_put(skb, 1), &code, 1);
+
+ memcpy(skb_put(skb, dlen), data, dlen);
+
+ return skb;
+}
+
+static inline void smp_send_cmd(struct l2cap_conn *conn, u8 code, u16 len, void *data)
+{
+ struct sk_buff *skb = smp_build_cmd(conn, code, len, data);
+
+ BT_DBG("code 0x%2.2x", code);
+
+ if (!skb)
+ return;
+
+ hci_send_acl(conn->hcon, skb, 0);
+}
+
+static int smp_conn_security(struct l2cap_conn *conn, __u8 sec_level)
+{
+ __u8 authreq;
+
+ BT_DBG("conn %p hcon %p level 0x%2.2x", conn, conn->hcon, sec_level);
+
+ switch (sec_level) {
+ case BT_SECURITY_MEDIUM:
+ /* Encrypted, no MITM protection */
+ authreq = 0x01;
+ break;
+
+ case BT_SECURITY_HIGH:
+ /* Bonding, MITM protection */
+ authreq = 0x05;
+ break;
+
+ case BT_SECURITY_LOW:
+ default:
+ return 1;
+ }
+
+ if (conn->hcon->link_mode & HCI_LM_MASTER) {
+ struct smp_cmd_pairing cp;
+ cp.io_capability = 0x00;
+ cp.oob_flag = 0x00;
+ cp.max_key_size = 16;
+ cp.init_key_dist = 0x00;
+ cp.resp_key_dist = 0x00;
+ cp.auth_req = authreq;
+ smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
+ } else {
+ struct smp_cmd_security_req cp;
+ cp.auth_req = authreq;
+ smp_send_cmd(conn, SMP_CMD_SECURITY_REQ, sizeof(cp), &cp);
+ }
+
+ return 0;
+}
+
/* Service level security */
static inline int l2cap_check_security(struct sock *sk)
{
@@ -4562,6 +4642,43 @@ done:
return 0;
}
+static inline void smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb)
+{
+ __u8 code = skb->data[0];
+ __u8 reason;
+
+ skb_pull(skb, 1);
+
+ switch (code) {
+ case SMP_CMD_PAIRING_REQ:
+ reason = SMP_PAIRING_NOTSUPP;
+ smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, 1, &reason);
+ l2cap_conn_del(conn->hcon, 0x05);
+ break;
+
+ case SMP_CMD_PAIRING_FAIL:
+ break;
+
+ case SMP_CMD_PAIRING_RSP:
+ case SMP_CMD_PAIRING_CONFIRM:
+ case SMP_CMD_PAIRING_RANDOM:
+ case SMP_CMD_ENCRYPT_INFO:
+ case SMP_CMD_MASTER_IDENT:
+ case SMP_CMD_IDENT_INFO:
+ case SMP_CMD_IDENT_ADDR_INFO:
+ case SMP_CMD_SIGN_INFO:
+ case SMP_CMD_SECURITY_REQ:
+ default:
+ BT_DBG("Unknown command code 0x%2.2x", code);
+
+ reason = SMP_CMD_NOTSUPP;
+ smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, 1, &reason);
+ l2cap_conn_del(conn->hcon, 0x05);
+ }
+
+ kfree_skb(skb);
+}
+
static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
{
struct l2cap_hdr *lh = (void *) skb->data;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 4/6] Bluetooth: Start SMP procedure
From: Anderson Briglia @ 2010-10-22 23:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Briglia, Vinicius Costa Gomes
Start SMP procedure for LE connections. This modification intercepts l2cap
received frames and call proper SMP functions to start the SMP procedure. By
now, no keys are being used.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
Signed-off-by: Anderson Briglia <anderson.briglia@openbossa.org>
---
net/bluetooth/l2cap.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index ba87c84..2a88f23 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -714,6 +714,8 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
l2cap_sock_clear_timer(sk);
sk->sk_state = BT_CONNECTED;
sk->sk_state_change(sk);
+ if (smp_conn_security(conn, l2cap_pi(sk)->sec_level))
+ BT_DBG("Insufficient security");
}
if (sk->sk_type != SOCK_SEQPACKET &&
@@ -4707,6 +4709,10 @@ static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
l2cap_conless_channel(conn, psm, skb);
break;
+ case L2CAP_CID_SMP:
+ smp_sig_channel(conn, skb);
+ break;
+
default:
l2cap_data_channel(conn, cid, skb);
break;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 5/6] Bluetooth: Fix initiated LE connections
From: Anderson Briglia @ 2010-10-22 23:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
Fix LE connections not being marked as master.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/hci_conn.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index c7309e4..1a48c6a 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -52,6 +52,7 @@ void hci_le_connect(struct hci_conn *conn)
conn->state = BT_CONNECT;
conn->out = 1;
+ conn->link_mode |= HCI_LM_MASTER;
memset(&cp, 0, sizeof(cp));
cp.scan_interval = cpu_to_le16(0x0004);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 6/6] Bluetooth: simple SMP pairing negotiation
From: Anderson Briglia @ 2010-10-22 23:57 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
This implementation only exchanges SMP messages between the Host and the
Remote. No keys are being generated. TK and STK generation will be
provided in further patches.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/l2cap.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 2a88f23..be91a33 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4644,6 +4644,104 @@ done:
return 0;
}
+static void smp_cmd_pairing_req(struct l2cap_conn *conn, struct sk_buff *skb)
+{
+ struct smp_cmd_pairing *rp = (void *) skb->data;
+
+ BT_DBG("");
+
+ skb_pull(skb, sizeof(struct smp_cmd_pairing));
+
+ rp->io_capability = 0x00;
+ rp->oob_flag = 0x00;
+ rp->max_key_size = 16;
+ rp->init_key_dist = 0x00;
+ rp->resp_key_dist = 0x00;
+ rp->auth_req &= 0x05;
+
+ smp_send_cmd(conn, SMP_CMD_PAIRING_RSP, sizeof(*rp), rp);
+}
+
+static void smp_cmd_pairing_rsp(struct l2cap_conn *conn, struct sk_buff *skb)
+{
+ struct smp_cmd_pairing_confirm cp;
+
+ BT_DBG("");
+
+ memset(&cp, 0, sizeof(struct smp_cmd_pairing_confirm));
+
+ smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(cp), &cp);
+}
+
+static void smp_cmd_pairing_confirm(struct l2cap_conn *conn,
+ struct sk_buff *skb)
+{
+ BT_DBG("");
+
+ if (conn->hcon->link_mode & HCI_LM_MASTER) {
+
+ struct smp_cmd_pairing_random random;
+
+ BT_DBG("master");
+
+ memset(&random, 0, sizeof(struct smp_cmd_pairing_random));
+
+ smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(random),
+ &random);
+ } else {
+ struct smp_cmd_pairing_confirm confirm;
+
+ BT_DBG("slave");
+
+ memset(&confirm, 0, sizeof(struct smp_cmd_pairing_confirm));
+
+ smp_send_cmd(conn, SMP_CMD_PAIRING_CONFIRM, sizeof(confirm),
+ &confirm);
+ }
+}
+
+static void smp_cmd_pairing_random(struct l2cap_conn *conn, struct sk_buff *skb)
+{
+ struct smp_cmd_pairing_random cp;
+
+ BT_DBG("");
+
+ skb_pull(skb, sizeof(struct smp_cmd_pairing_random));
+
+ /* FIXME: check if random matches */
+
+ if (conn->hcon->link_mode & HCI_LM_MASTER) {
+ BT_DBG("master");
+ /* FIXME: start encryption */
+ } else {
+ BT_DBG("slave");
+
+ memset(&cp, 0, sizeof(struct smp_cmd_pairing_random));
+
+ smp_send_cmd(conn, SMP_CMD_PAIRING_RANDOM, sizeof(cp), &cp);
+ }
+}
+
+static void smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb)
+{
+ struct smp_cmd_security_req *rp = (void *) skb->data;
+ struct smp_cmd_pairing cp;
+
+ BT_DBG("");
+
+ skb_pull(skb, sizeof(struct smp_cmd_security_req));
+ memset(&cp, 0, sizeof(struct smp_cmd_pairing));
+
+ cp.io_capability = 0x00;
+ cp.oob_flag = 0x00;
+ cp.max_key_size = 16;
+ cp.init_key_dist = 0x00;
+ cp.resp_key_dist = 0x00;
+ cp.auth_req = rp->auth_req & 0x05;
+
+ smp_send_cmd(conn, SMP_CMD_PAIRING_REQ, sizeof(cp), &cp);
+}
+
static inline void smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb)
{
__u8 code = skb->data[0];
@@ -4651,25 +4749,37 @@ static inline void smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb)
skb_pull(skb, 1);
+ BT_DBG("conn %p code 0x%2.2x", conn, code);
+
switch (code) {
case SMP_CMD_PAIRING_REQ:
- reason = SMP_PAIRING_NOTSUPP;
- smp_send_cmd(conn, SMP_CMD_PAIRING_FAIL, 1, &reason);
- l2cap_conn_del(conn->hcon, 0x05);
+ smp_cmd_pairing_req(conn, skb);
break;
case SMP_CMD_PAIRING_FAIL:
break;
case SMP_CMD_PAIRING_RSP:
+ smp_cmd_pairing_rsp(conn, skb);
+ break;
+
+ case SMP_CMD_SECURITY_REQ:
+ smp_cmd_security_req(conn, skb);
+ break;
+
case SMP_CMD_PAIRING_CONFIRM:
+ smp_cmd_pairing_confirm(conn, skb);
+ break;
+
case SMP_CMD_PAIRING_RANDOM:
+ smp_cmd_pairing_random(conn, skb);
+ break;
+
case SMP_CMD_ENCRYPT_INFO:
case SMP_CMD_MASTER_IDENT:
case SMP_CMD_IDENT_INFO:
case SMP_CMD_IDENT_ADDR_INFO:
case SMP_CMD_SIGN_INFO:
- case SMP_CMD_SECURITY_REQ:
default:
BT_DBG("Unknown command code 0x%2.2x", code);
--
1.7.0.4
^ permalink raw reply related
* [PATCH] This patch avoids a crash when org.bluez.Manager GetProperties request is
From: Tommi Keisala @ 2010-10-23 5:33 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <20101022131207.GA29655 () jh-x301>
I made changes requested
^ permalink raw reply
* [PATCH] This patch avoids a crash when org.bluez.Manager GetProperties request is received and there is not yet any adapters ready. Happens often for example when bluetoothd and ofonod is started next ot each other.
From: Tommi Keisala @ 2010-10-23 5:33 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tommi Keisala
In-Reply-To: <20101022131207.GA29655 () jh-x301>
---
src/manager.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/manager.c b/src/manager.c
index aff069c..dd9560f 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -197,13 +197,14 @@ static DBusMessage *get_properties(DBusConnection *conn,
DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
array = g_new0(char *, g_slist_length(adapters) + 1);
- for (i = 0, list = adapters; list; list = list->next, i++) {
+ for (i = 0, list = adapters; list; list = list->next) {
struct btd_adapter *adapter = list->data;
if (!adapter_is_ready(adapter))
continue;
array[i] = (char *) adapter_get_path(adapter);
+ i++;
}
dict_append_array(&dict, "Adapters", DBUS_TYPE_OBJECT_PATH, &array, i);
g_free(array);
--
1.7.0.4
^ permalink raw reply related
* Re: (Health) ChannelConnected signal when MDL aborted?
From: Santiago Carot @ 2010-10-23 7:50 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: Jose Antonio Santos Cadenas, linux-bluetooth
In-Reply-To: <295EA951-2357-4B6B-B558-0B3F1D6FFDD9@signove.com>
Hi,
2010/10/22 Elvis Pfützenreuter <epx@signove.com>:
> Taking a look in the code, I saw that hdp_mcap_mdl_aborted_cb() emits a ChannelConnected signal.
>
> I understand that after an abort, the MDL still exists, and can be reconnected, so I understand the intention of informing the application that a channel has been "created". But I'm not sure that it is opportune.
>
> The first thing the application will do is trying to Acquire() the channel's file descriptor (that does not exist) and that triggers a reconnection attempt, from acceptor to initiator. While the most sensible thing (in my view) is waiting for the initiator to reconnect -- if it aborted, it must have a good reason, and it will probably reject the immediate call back.
>
You are forgetting that there may be more than one health applications
running over HDP at the same time, if one of them creates a data
channel, that data data channel will be exist at MCAP level even if
the initiator abort the connection. If other application wants to
create a new data channel with the same configuration, it may be want
reconnect that data channel avoiding to create another data channel.
It is a best practise wich is recomended in MCAP and best explained in
the Health Device Profile white paper document to reduce the amount of
data interchanged between medical devices (in IEEE/11073-20601
terminology: "agents" and "managers"). Remember that channels may be
shared between applications.
That is the reason why HDP sends out a signal when a data channel has
been created.
Of course, the efficence of the protocol will depend on intelligence
of the programmer of the applications to take advantages of MCAP
reconnections. In any case, you will need to know when a data channel
has been created over the MCL (by you, or by other application).
Regards,
Santiago
^ permalink raw reply
* Re: (Health) ChannelConnected signal when MDL aborted?
From: Santiago Carot @ 2010-10-23 8:00 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: Jose Antonio Santos Cadenas, linux-bluetooth
In-Reply-To: <AANLkTi=65C7HQXQW1Bqbgf4sNY3wbV2CzqoENBc-b04g@mail.gmail.com>
Hi Elvis,
2010/10/23 Santiago Carot <sancane@gmail.com>:
> Hi,
>
> 2010/10/22 Elvis Pfützenreuter <epx@signove.com>:
>> Taking a look in the code, I saw that hdp_mcap_mdl_aborted_cb() emits a ChannelConnected signal.
>>
>> I understand that after an abort, the MDL still exists, and can be reconnected, so I understand the intention of informing the application that a channel has been "created". But I'm not sure that it is opportune.
>>
>> The first thing the application will do is trying to Acquire() the channel's file descriptor (that does not exist) and that triggers a reconnection attempt, from acceptor to initiator. While the most sensible thing (in my view) is waiting for the initiator to reconnect -- if it aborted, it must have a good reason, and it will probably reject the immediate call back.
>>
>
> You are forgetting that there may be more than one health applications
> running over HDP at the same time, if one of them creates a data
> channel, that data data channel will be exist at MCAP level even if
> the initiator abort the connection. If other application wants to
> create a new data channel with the same configuration, it may be want
> reconnect that data channel avoiding to create another data channel.
> It is a best practise wich is recomended in MCAP and best explained in
> the Health Device Profile white paper document to reduce the amount of
> data interchanged between medical devices (in IEEE/11073-20601
> terminology: "agents" and "managers"). Remember that channels may be
> shared between applications.
>
I found the example that I was trying to explain here in page 16 of
the HDP whitepaper, there you can see two applications over HDP
sharing the same data channel (one of them providing support to
diferents device specializations).
> That is the reason why HDP sends out a signal when a data channel has
> been created.
> Of course, the efficence of the protocol will depend on intelligence
> of the programmer of the applications to take advantages of MCAP
> reconnections. In any case, you will need to know when a data channel
> has been created over the MCL (by you, or by other application).
>
> Regards,
>
> Santiago
>
^ permalink raw reply
* Re: (Health) ChannelConnected signal when MDL aborted?
From: Santiago Carot @ 2010-10-23 8:25 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: Jose Antonio Santos Cadenas, linux-bluetooth
In-Reply-To: <AANLkTimvQH4jA=AudrGA7Sj1vAFB3OjocOur2J2=Yy2f@mail.gmail.com>
Hi again,
2010/10/23 Santiago Carot <sancane@gmail.com>:
> Hi Elvis,
>
> 2010/10/23 Santiago Carot <sancane@gmail.com>:
>> Hi,
>>
>> 2010/10/22 Elvis Pfützenreuter <epx@signove.com>:
>>> Taking a look in the code, I saw that hdp_mcap_mdl_aborted_cb() emits a ChannelConnected signal.
>>>
>>> I understand that after an abort, the MDL still exists, and can be reconnected, so I understand the intention of informing the application that a channel has been "created". But I'm not sure that it is opportune.
>>>
>>> The first thing the application will do is trying to Acquire() the channel's file descriptor (that does not exist) and that triggers a reconnection attempt, from acceptor to initiator. While the most sensible thing (in my view) is waiting for the initiator to reconnect -- if it aborted, it must have a good reason, and it will probably reject the immediate call back.
>>>
>>
>> You are forgetting that there may be more than one health applications
>> running over HDP at the same time, if one of them creates a data
>> channel, that data data channel will be exist at MCAP level even if
>> the initiator abort the connection. If other application wants to
>> create a new data channel with the same configuration, it may be want
>> reconnect that data channel avoiding to create another data channel.
>> It is a best practise wich is recomended in MCAP and best explained in
>> the Health Device Profile white paper document to reduce the amount of
>> data interchanged between medical devices (in IEEE/11073-20601
>> terminology: "agents" and "managers"). Remember that channels may be
>> shared between applications.
>>
>
> I found the example that I was trying to explain here in page 16 of
> the HDP whitepaper, there you can see two applications over HDP
> sharing the same data channel (one of them providing support to
> diferents device specializations).
>
Well, may be that above reference is not the best because in the
picture applications share the same MDEP too, but I think that it is
enough to see analogies.
>> That is the reason why HDP sends out a signal when a data channel has
>> been created.
>> Of course, the efficence of the protocol will depend on intelligence
>> of the programmer of the applications to take advantages of MCAP
>> reconnections. In any case, you will need to know when a data channel
>> has been created over the MCL (by you, or by other application).
>>
>> Regards,
>>
>> Santiago
>>
>
^ permalink raw reply
* Re: [PATCH 1/1] Fix small coding style issues
From: Johan Hedberg @ 2010-10-23 12:28 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <1287775122-12536-1-git-send-email-epx@signove.com>
Hi Elvis,
On Fri, Oct 22, 2010, Elvis Pfützenreuter wrote:
> Based on http://lxr.linux.no/linux+v2.6.36/Documentation/CodingStyle#L171
> ---
> health/hdp_util.c | 24 +++++++++++++-----------
> health/mcap_lib.h | 8 ++++----
> 2 files changed, 17 insertions(+), 15 deletions(-)
Thanks, I've pushed the patch upstream. FWIW, The BlueZ code base
doesn't really enforce the kernel style rule of having braces for all
parts of if-else statements when at least one branch has them.
Johan
^ permalink raw reply
* Re: [PATCH] This patch avoids a crash when org.bluez.Manager GetProperties request is received and there is not yet any adapters ready. Happens often for example when bluetoothd and ofonod is started next ot each other.
From: Johan Hedberg @ 2010-10-23 12:31 UTC (permalink / raw)
To: Tommi Keisala; +Cc: linux-bluetooth
In-Reply-To: <1287811998-20490-2-git-send-email-ext-tommi.keisala@nokia.com>
Hi Tommi,
On Sat, Oct 23, 2010, Tommi Keisala wrote:
> ---
> src/manager.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
You seem to have somehow managed to screw up the commit message this
time: everything in the summary line and nothing in the message body.
Anyway, to avoid further feedback rounds (since the patch itself is fine
now) I went ahead and fixed the commit message and pushed your patch
upstream. Thanks.
Johan
^ permalink raw reply
* Re: AVRCP 1.4 : Future on Target Role
From: João Paulo Rechi Vita @ 2010-10-23 15:50 UTC (permalink / raw)
To: linux-bluetooth
Cc: Sander van Grieken, Shivendra Agrawal, David Stockwell,
Waldemar Rymarkiewicz, Luiz Augusto von Dentz, Johan Hedberg
In-Reply-To: <AANLkTikgKM1MNpwP2JL1GdERZ60dE6dkFNC=rdyQDhOV@mail.gmail.com>
Hello all,
I've seen some discuss regarding how to add support for AVRCP 1.3 and
1.4 versions on the ML lately, and some expectations over the related
work I've done in BlueZ. Sorry for the long delay on replying, I've
been pretty busy lately and just didn't got the time. I'm planing to
put some effort again on this work on the coming weeks.
On Tue, Oct 19, 2010 at 14:14, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi,
>
> On Tue, Oct 19, 2010 at 12:47 PM, Sander van Grieken
> <sander@outrightsolutions.nl> wrote:
>> I am very much in favor of not directly depending on MPRIS, but instead letting
>> applications registering themselves as a target. For two reasons:
>>
>> - AVRCP seems to be a superset of MPRIS (which is very limited IMO), and might have
>> different semantics, especially w.r.t. event notifications. So we would limit ourselves to
>> the intersecting subset of both technologies.
>> - A separate AVRCP/TG <-> MPRIS bridge agent would still allow controlling all MPRIS-
>> enabled players, so we can have both full implementation of the AVRCP spec, AND generic
>> MPRIS support.
>
> Sounds good to me, we can use Media API to register players as well.
>
I agree with Sander's opinion that MPRIS seems a bit limited for 1.4.
OTOH it's an already defined and under-adoption standard. If we have
applications registering themselves directly with us, we'll have to
define a new interface for that and push this to the player
applications. Do you guys already have a draft of these interfaces
(for the applications and extensions to the Media API)? I guess we
should try to define exactly what we need first, and them see what's
missing from MPRIS.
>>> > I am keen to receive your feedback with some ideas and thoughts to put
>>> > my effort in right direction.
>>> >
>>> > Question:
>>> > Is anyone working on AVRCP 1.4 Target role profile development?
>>>
>>> Joao Paulo (http://jprvita.wordpress.com/2010/07/22/avrcp-metadata/)
>>
>> Actually, the metadata work is not part of v1.4 of the spec, but 1.3
>>
>> Second, I have already added some boilerplate stuff (like a DBUS Connect method for RCP and
>> some fixes for CT commands), but I've based on Joao's branch, so I have to wait until his
>> stuff gets merged. Alternatively, I could rebase that stuff on HEAD, but that would overlap
>> and conflict with Joao's stuff, so I'm hesitant to go there.
>
Have you published this code somewhere?
> I hope to see this code being push upstream soon, I will try to
> contact Joao Paulo to get an idea if the code is ready to be
> reviewed/pushed so we get the things rolling.
>
I'm finally here :) I've focused mostly in the 1.3 version of the spec
and having pulseaudio as a middle-man in my work, and have written
most of the code in pulseaudio to handle these new functionality. Even
if we take the approach described above for 1.4, IMO it makes sense to
upstream this so we can have earlier support for 1.3 and also to
support Sander's work. Luiz, Johan, what do you think?
I'll review the code once more, since I've written it a few months
ago, but my main concern about pushing it upstream right now is that
it hasn't been tested yet. I have no device to test against and PTS
can't give us no guarantee whether the code really works in practice
or not (but it's still better than nothing). Sander, David, have you
tested my code against any real device?
--
João Paulo Rechi Vita
http://jprvita.wordpress.com/
^ permalink raw reply
* Re: (Health) ChannelConnected signal when MDL aborted?
From: Elvis Pfützenreuter @ 2010-10-23 20:35 UTC (permalink / raw)
To: Santiago Carot; +Cc: Jose Antonio Santos Cadenas, linux-bluetooth
In-Reply-To: <AANLkTi=65C7HQXQW1Bqbgf4sNY3wbV2CzqoENBc-b04g@mail.gmail.com>
> You are forgetting that there may be more than one health applications
> running over HDP at the same time, if one of them creates a data
> channel, that data data channel will be exist at MCAP level even if
> the initiator abort the connection. If other application wants to
> create a new data channel with the same configuration, it may be want
> reconnect that data channel avoiding to create another data channel.
> It is a best practise wich is recomended in MCAP and best explained in
> the Health Device Profile white paper document to reduce the amount of
> data interchanged between medical devices (in IEEE/11073-20601
> terminology: "agents" and "managers"). Remember that channels may be
> shared between applications.
I'm still not convinced :) I can't see the point of sharing a HealthChannel that is not healthy (pardon the joke).
Maybe I'm being too pragmatic, but I expect that most real HDP devices will be like that Nonin oximeter: incapable of reconnecting MDLs and incapable of accepting connections. Sharing a channel in that situation will make spend much more, not less, energy, because applications will Acquire() and trigger a reconnection over a channel that actually does not exist and wouldn't be accepted even if it existed.
Anyway, MDL aborts are expected to be rare, so the actual problem won't be seen often in practice.
I think we have another, more important problem: naming the channel after MDLID.
For example, the channel that comes from oximeter has always the same path here: /org/bluez/19750/hci0/dev_00_1C_05_00_28_85/chan1. It is incapable of reconnections but always recreates the MDL with the same MDL ID = 1.
Now, let's suppose a MDL is created, destroyed and re-created, but for some reason the application the application takes too long to act upon the signal. You have three signals:
ChannelConnected chan1
ChannelDeleted chan1
ChannelConnected chan1 (a second channel with the same MDL ID)
Application will Acquire() on first signal, thinking it is getting the FD for the first instance of chan1, but it will actually get the FD for the second "version" of chan1.
Then it will process the ChannelDeleted signal, closing a perfectly good channel. And then it sees the third signal, and tries the Acquire() the fd it has just closed. If the other side tries to reconnect immediately but does not support MCAP reconnections, this could go on forever.
A possible solution is to use a monotonic number for the channel path suffix, without relation to MDL ID.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox