* [PATCH 3/5] Use GAttrib buffer for ATT protocol PDUs
From: Bruna Moreira @ 2011-03-16 11:25 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1300274712-3931-1-git-send-email-bruna.moreira@openbossa.org>
Prior to this commit, there were local buffers inside GATT functions.
Now, a single buffer is used, to make sure the MTU limit is respected.
---
attrib/gatt.c | 109 +++++++++++++++++++++++++++++++++-----------------------
1 files changed, 64 insertions(+), 45 deletions(-)
diff --git a/attrib/gatt.c b/attrib/gatt.c
index 32bd4a0..d3333fd 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -108,9 +108,9 @@ static void primary_by_uuid_cb(guint8 status, const guint8 *ipdu,
struct discover_primary *dp = user_data;
GSList *ranges, *last;
struct att_range *range;
- uint8_t opdu[ATT_DEFAULT_LE_MTU];
+ uint8_t *buf;
guint16 oplen;
- int err = 0;
+ int err = 0, buflen;
if (status) {
err = status == ATT_ECODE_ATTR_NOT_FOUND ? 0 : status;
@@ -129,13 +129,14 @@ static void primary_by_uuid_cb(guint8 status, const guint8 *ipdu,
if (range->end == 0xffff)
goto done;
+ buf = g_attrib_get_buffer(dp->attrib, &buflen);
oplen = encode_discover_primary(range->end + 1, 0xffff, &dp->uuid,
- opdu, sizeof(opdu));
+ buf, buflen);
if (oplen == 0)
goto done;
- g_attrib_send(dp->attrib, 0, opdu[0], opdu, oplen, primary_by_uuid_cb,
+ g_attrib_send(dp->attrib, 0, buf[0], buf, oplen, primary_by_uuid_cb,
dp, NULL);
return;
@@ -196,12 +197,13 @@ static void primary_all_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
err = 0;
if (end != 0xffff) {
- uint8_t opdu[ATT_DEFAULT_LE_MTU];
+ int buflen;
+ uint8_t *buf = g_attrib_get_buffer(dp->attrib, &buflen);
guint16 oplen = encode_discover_primary(end + 1, 0xffff, NULL,
- opdu, sizeof(opdu));
+ buf, buflen);
- g_attrib_send(dp->attrib, 0, opdu[0], opdu, oplen,
- primary_all_cb, dp, NULL);
+ g_attrib_send(dp->attrib, 0, buf[0], buf, oplen, primary_all_cb,
+ dp, NULL);
return;
}
@@ -215,11 +217,12 @@ guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
gpointer user_data)
{
struct discover_primary *dp;
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
+ int buflen;
+ uint8_t *buf = g_attrib_get_buffer(attrib, &buflen);
GAttribResultFunc cb;
guint16 plen;
- plen = encode_discover_primary(0x0001, 0xffff, uuid, pdu, sizeof(pdu));
+ plen = encode_discover_primary(0x0001, 0xffff, uuid, buf, buflen);
if (plen == 0)
return 0;
@@ -237,7 +240,7 @@ guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
} else
cb = primary_all_cb;
- return g_attrib_send(attrib, 0, pdu[0], pdu, plen, cb, dp, NULL);
+ return g_attrib_send(attrib, 0, buf[0], buf, plen, cb, dp, NULL);
}
static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
@@ -246,7 +249,8 @@ static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
struct discover_char *dc = user_data;
struct att_data_list *list;
unsigned int i, err;
- uint8_t opdu[ATT_DEFAULT_LE_MTU];
+ int buflen;
+ uint8_t *buf;
guint16 oplen;
bt_uuid_t uuid;
uint16_t last = 0;
@@ -293,15 +297,17 @@ static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
err = 0;
if (last != 0) {
+ buf = g_attrib_get_buffer(dc->attrib, &buflen);
+
bt_uuid16_create(&uuid, GATT_CHARAC_UUID);
- oplen = enc_read_by_type_req(last + 1, dc->end, &uuid, opdu,
- sizeof(opdu));
+ oplen = enc_read_by_type_req(last + 1, dc->end, &uuid, buf,
+ buflen);
if (oplen == 0)
return;
- g_attrib_send(dc->attrib, 0, opdu[0], opdu, oplen,
+ g_attrib_send(dc->attrib, 0, buf[0], buf, oplen,
char_discovered_cb, dc, NULL);
return;
@@ -315,14 +321,15 @@ done:
guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
gatt_cb_t func, gpointer user_data)
{
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
+ int buflen;
+ uint8_t *buf = g_attrib_get_buffer(attrib, &buflen);
struct discover_char *dc;
guint16 plen;
bt_uuid_t uuid;
bt_uuid16_create(&uuid, GATT_CHARAC_UUID);
- plen = enc_read_by_type_req(start, end, &uuid, pdu, sizeof(pdu));
+ plen = enc_read_by_type_req(start, end, &uuid, buf, buflen);
if (plen == 0)
return 0;
@@ -335,7 +342,7 @@ guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
dc->user_data = user_data;
dc->end = end;
- return g_attrib_send(attrib, 0, pdu[0], pdu, plen, char_discovered_cb,
+ return g_attrib_send(attrib, 0, buf[0], buf, plen, char_discovered_cb,
dc, NULL);
}
@@ -343,15 +350,16 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
bt_uuid_t *uuid, GAttribResultFunc func,
gpointer user_data)
{
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
+ int buflen;
+ uint8_t *buf = g_attrib_get_buffer(attrib, &buflen);
guint16 plen;
- plen = enc_read_by_type_req(start, end, uuid, pdu, sizeof(pdu));
+ plen = enc_read_by_type_req(start, end, uuid, buf, buflen);
if (plen == 0)
return 0;
return g_attrib_send(attrib, 0, ATT_OP_READ_BY_TYPE_REQ,
- pdu, plen, func, user_data, NULL);
+ buf, plen, func, user_data, NULL);
}
struct read_long_data {
@@ -382,7 +390,8 @@ static void read_blob_helper(guint8 status, const guint8 *rpdu, guint16 rlen,
gpointer user_data)
{
struct read_long_data *long_read = user_data;
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
+ uint8_t *buf;
+ int buflen;
guint8 *tmp;
guint16 plen;
guint id;
@@ -403,13 +412,14 @@ static void read_blob_helper(guint8 status, const guint8 *rpdu, guint16 rlen,
long_read->buffer = tmp;
long_read->size += rlen - 1;
- if (rlen < ATT_DEFAULT_LE_MTU)
+ buf = g_attrib_get_buffer(long_read->attrib, &buflen);
+ if (rlen < buflen)
goto done;
plen = enc_read_blob_req(long_read->handle, long_read->size - 1,
- pdu, sizeof(pdu));
+ buf, buflen);
id = g_attrib_send(long_read->attrib, long_read->id,
- ATT_OP_READ_BLOB_REQ, pdu, plen,
+ ATT_OP_READ_BLOB_REQ, buf, plen,
read_blob_helper, long_read, read_long_destroy);
if (id != 0) {
@@ -428,11 +438,12 @@ static void read_char_helper(guint8 status, const guint8 *rpdu,
guint16 rlen, gpointer user_data)
{
struct read_long_data *long_read = user_data;
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
+ int buflen;
+ uint8_t *buf = g_attrib_get_buffer(long_read->attrib, &buflen);
guint16 plen;
guint id;
- if (status != 0 || rlen < ATT_DEFAULT_LE_MTU)
+ if (status != 0 || rlen < buflen)
goto done;
long_read->buffer = g_malloc(rlen);
@@ -443,9 +454,9 @@ static void read_char_helper(guint8 status, const guint8 *rpdu,
memcpy(long_read->buffer, rpdu, rlen);
long_read->size = rlen;
- plen = enc_read_blob_req(long_read->handle, rlen - 1, pdu, sizeof(pdu));
+ plen = enc_read_blob_req(long_read->handle, rlen - 1, buf, buflen);
id = g_attrib_send(long_read->attrib, long_read->id,
- ATT_OP_READ_BLOB_REQ, pdu, plen, read_blob_helper,
+ ATT_OP_READ_BLOB_REQ, buf, plen, read_blob_helper,
long_read, read_long_destroy);
if (id != 0) {
@@ -462,7 +473,8 @@ done:
guint gatt_read_char(GAttrib *attrib, uint16_t handle, uint16_t offset,
GAttribResultFunc func, gpointer user_data)
{
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
+ uint8_t *buf;
+ int buflen;
guint16 plen;
guint id;
struct read_long_data *long_read;
@@ -477,14 +489,15 @@ guint gatt_read_char(GAttrib *attrib, uint16_t handle, uint16_t offset,
long_read->user_data = user_data;
long_read->handle = handle;
+ buf = g_attrib_get_buffer(attrib, &buflen);
if (offset > 0) {
- plen = enc_read_blob_req(long_read->handle, offset, pdu,
- sizeof(pdu));
- id = g_attrib_send(attrib, 0, ATT_OP_READ_BLOB_REQ, pdu, plen,
+ plen = enc_read_blob_req(long_read->handle, offset, buf,
+ buflen);
+ id = g_attrib_send(attrib, 0, ATT_OP_READ_BLOB_REQ, buf, plen,
read_blob_helper, long_read, read_long_destroy);
} else {
- plen = enc_read_req(handle, pdu, sizeof(pdu));
- id = g_attrib_send(attrib, 0, ATT_OP_READ_REQ, pdu, plen,
+ plen = enc_read_req(handle, buf, buflen);
+ id = g_attrib_send(attrib, 0, ATT_OP_READ_REQ, buf, plen,
read_char_helper, long_read, read_long_destroy);
}
@@ -501,39 +514,45 @@ guint gatt_read_char(GAttrib *attrib, uint16_t handle, uint16_t offset,
guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
int vlen, GAttribResultFunc func, gpointer user_data)
{
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
+ uint8_t *buf;
+ int buflen;
guint16 plen;
+ buf = g_attrib_get_buffer(attrib, &buflen);
if (func)
- plen = enc_write_req(handle, value, vlen, pdu, sizeof(pdu));
+ plen = enc_write_req(handle, value, vlen, buf, buflen);
else
- plen = enc_write_cmd(handle, value, vlen, pdu, sizeof(pdu));
+ plen = enc_write_cmd(handle, value, vlen, buf, buflen);
- return g_attrib_send(attrib, 0, pdu[0], pdu, plen, func,
+ return g_attrib_send(attrib, 0, buf[0], buf, plen, func,
user_data, NULL);
}
guint gatt_find_info(GAttrib *attrib, uint16_t start, uint16_t end,
GAttribResultFunc func, gpointer user_data)
{
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
+ uint8_t *buf;
+ int buflen;
guint16 plen;
- plen = enc_find_info_req(start, end, pdu, sizeof(pdu));
+ buf = g_attrib_get_buffer(attrib, &buflen);
+ plen = enc_find_info_req(start, end, buf, buflen);
if (plen == 0)
return 0;
- return g_attrib_send(attrib, 0, ATT_OP_FIND_INFO_REQ, pdu, plen, func,
+ return g_attrib_send(attrib, 0, ATT_OP_FIND_INFO_REQ, buf, plen, func,
user_data, NULL);
}
guint gatt_write_cmd(GAttrib *attrib, uint16_t handle, uint8_t *value, int vlen,
GDestroyNotify notify, gpointer user_data)
{
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
+ uint8_t *buf;
+ int buflen;
guint16 plen;
- plen = enc_write_cmd(handle, value, vlen, pdu, sizeof(pdu));
- return g_attrib_send(attrib, 0, ATT_OP_WRITE_CMD, pdu, plen, NULL,
+ buf = g_attrib_get_buffer(attrib, &buflen);
+ plen = enc_write_cmd(handle, value, vlen, buf, buflen);
+ return g_attrib_send(attrib, 0, ATT_OP_WRITE_CMD, buf, plen, NULL,
user_data, notify);
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/5] Add internal buffer to GAttrib struct
From: Bruna Moreira @ 2011-03-16 11:25 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1300274712-3931-1-git-send-email-bruna.moreira@openbossa.org>
The new buffer is allocated in g_attrib_new() and it will be used to
send/receive PDUs. The buffer size is the MTU read from L2CAP channel
limited to ATT_MAX_MTU. Functions to handle the buffer size were also
created.
---
attrib/gattrib.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
attrib/gattrib.h | 3 +++
2 files changed, 49 insertions(+), 0 deletions(-)
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 07e56de..ba168e7 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -40,6 +40,8 @@
struct _GAttrib {
GIOChannel *io;
gint refs;
+ uint8_t *buf;
+ int buflen;
guint read_watch;
guint write_watch;
guint timeout_watch;
@@ -193,6 +195,8 @@ static void attrib_destroy(GAttrib *attrib)
g_io_channel_unref(attrib->io);
}
+ g_free(attrib->buf);
+
if (attrib->destroy)
attrib->destroy(attrib->destroy_user_data);
@@ -386,6 +390,7 @@ done:
GAttrib *g_attrib_new(GIOChannel *io)
{
struct _GAttrib *attrib;
+ uint16_t omtu;
g_io_channel_set_encoding(io, NULL, NULL);
g_io_channel_set_buffered(io, FALSE);
@@ -401,6 +406,17 @@ GAttrib *g_attrib_new(GIOChannel *io)
G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
received_data, attrib);
+ if (bt_io_get(attrib->io, BT_IO_L2CAP, NULL,
+ BT_IO_OPT_OMTU, &omtu,
+ BT_IO_OPT_INVALID)) {
+ if (omtu > ATT_MAX_MTU)
+ omtu = ATT_MAX_MTU;
+ } else
+ omtu = ATT_DEFAULT_LE_MTU;
+
+ attrib->buf = g_malloc0(omtu);
+ attrib->buflen = omtu;
+
return g_attrib_ref(attrib);
}
@@ -504,6 +520,36 @@ gboolean g_attrib_set_debug(GAttrib *attrib,
return TRUE;
}
+uint8_t *g_attrib_get_buffer(GAttrib *attrib, int *len)
+{
+ if (len == NULL)
+ return NULL;
+
+ *len = attrib->buflen;
+
+ return attrib->buf;
+}
+
+gboolean g_attrib_set_buffer(GAttrib *attrib, int mtu)
+{
+ if (mtu < ATT_DEFAULT_LE_MTU)
+ mtu = ATT_DEFAULT_LE_MTU;
+
+ if (mtu > ATT_MAX_MTU)
+ mtu = ATT_MAX_MTU;
+
+ if (!bt_io_set(attrib->io, BT_IO_L2CAP, NULL,
+ BT_IO_OPT_OMTU, mtu,
+ BT_IO_OPT_INVALID))
+ return FALSE;
+
+ attrib->buf = g_realloc(attrib->buf, mtu);
+
+ attrib->buflen = mtu;
+
+ return TRUE;
+}
+
guint g_attrib_register(GAttrib *attrib, guint8 opcode,
GAttribNotifyFunc func, gpointer user_data,
GDestroyNotify notify)
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index f25208d..0211068 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -68,6 +68,9 @@ guint g_attrib_register(GAttrib *attrib, guint8 opcode,
gboolean g_attrib_is_encrypted(GAttrib *attrib);
+uint8_t *g_attrib_get_buffer(GAttrib *attrib, int *len);
+gboolean g_attrib_set_buffer(GAttrib *attrib, int mtu);
+
gboolean g_attrib_unregister(GAttrib *attrib, guint id);
gboolean g_attrib_unregister_all(GAttrib *attrib);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 1/5] TODO: remove 'fix MTU exchange' task
From: Bruna Moreira @ 2011-03-16 11:25 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bruna Moreira
---
TODO | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/TODO b/TODO
index 1120ad9..8b92808 100644
--- a/TODO
+++ b/TODO
@@ -136,11 +136,6 @@ ATT/GATT
Priority: Medium
Complexity: C2
-- GATT server: fix MTU exchange
-
- Priority: Medium
- Complexity: C2
-
- Implement ATT PDU validation. Malformed PDUs can cause division by zero
when decoding PDUs. A proper error PDU should be returned for this case.
See decoding function in att.c file.
--
1.7.0.4
^ permalink raw reply related
* RE: [PATCH v4 2/6] Sim Access Profile Server
From: Waldemar.Rymarkiewicz @ 2011-03-16 11:04 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <20110316094003.GA11136@jh-x301>
>Use shorter define names (since they really are quite long),
>move the checks into separate functions, e.g.
>validate_set_transport_protocol_req(),
>or redo the if-statement to something like:
>
> if (msg->nparam != 0x01)
> return -EBADMSG;
> if (msg->param->id != SAP_PARAM_ID_TRANSPORT_PROTOCOL)
> return -EBADMSG;
> if (ntohs(msg->param->len) !=
>SAP_PARAM_ID_TRANSPORT_PROTOCOL_LEN)
> return -EBADMSG;
> ...
> return 0;
>
>It becomes much easier for a human to parse it that way since
>you get to process the individual conditions clearly one at a time.
>
Thanks. I will rearrange if-statements now. I agree, however, defines are to long and will change it in a separate patch not to introduce a mess. The defines are used in already upstream code.
Will send v5 in a while.
Waldek
^ permalink raw reply
* Re: [PATCH 3/3] Bluetooth: mgmt: Add support for setting the local name
From: Szymon Janc @ 2011-03-16 10:09 UTC (permalink / raw)
To: johan.hedberg@gmail.com; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1300215293-30294-3-git-send-email-johan.hedberg@gmail.com>
> From: Johan Hedberg <johan.hedberg@nokia.com>
>
> This patch adds a new set_local_name management command as well as a
> local_name_changed management event. With these user space can both
> change the local name as well as monitor changes to it by others.
>
> The management messages reserve 249 bytes for the name instead of 248
> (like in the HCI spec) so that there is always a guarantee that it is
> nul-terminated. That way it can safely be passed onto string
> manipulation functions.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> include/net/bluetooth/mgmt.h | 10 +++++
> net/bluetooth/hci_event.c | 9 +++-
> net/bluetooth/mgmt.c | 76 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 93 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 441dadb..c77c082 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -767,6 +767,7 @@ int mgmt_user_confirm_reply_complete(u16 index, bdaddr_t *bdaddr, u8 status);
> int mgmt_user_confirm_neg_reply_complete(u16 index, bdaddr_t *bdaddr,
> u8 status);
> int mgmt_auth_failed(u16 index, bdaddr_t *bdaddr, u8 status);
> +int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status);
>
> /* HCI info for socket */
> #define hci_pi(sk) ((struct hci_pinfo *) sk)
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 4deeea1..f7d4df3 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -168,6 +168,11 @@ struct mgmt_rp_user_confirm_reply {
>
> #define MGMT_OP_USER_CONFIRM_NEG_REPLY 0x0016
>
> +#define MGMT_OP_SET_LOCAL_NAME 0x0017
> +struct mgmt_cp_set_local_name {
> + __u8 name[249];
> +} __packed;
> +
> #define MGMT_EV_CMD_COMPLETE 0x0001
> struct mgmt_ev_cmd_complete {
> __le16 opcode;
> @@ -235,3 +240,8 @@ struct mgmt_ev_auth_failed {
> bdaddr_t bdaddr;
> __u8 status;
> } __packed;
> +
> +#define MGMT_EV_LOCAL_NAME_CHANGED 0x0011
> +struct mgmt_ev_local_name_changed {
> + __u8 name[249];
> +} __packed;
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 3fbfa50..e7f7cb0 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -193,13 +193,16 @@ static void hci_cc_write_local_name(struct hci_dev *hdev, struct sk_buff *skb)
>
> BT_DBG("%s status 0x%x", hdev->name, status);
>
> - if (status)
> - return;
> -
> sent = hci_sent_cmd_data(hdev, HCI_OP_WRITE_LOCAL_NAME);
> if (!sent)
> return;
>
> + if (test_bit(HCI_MGMT, &hdev->flags))
> + mgmt_set_local_name_complete(hdev->id, sent, status);
> +
> + if (status)
> + return;
> +
> memcpy(hdev->dev_name, sent, 248);
> }
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 67529c8..6e84ab2 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1258,6 +1258,46 @@ failed:
> return err;
> }
>
> +static int set_local_name(struct sock *sk, u16 index, unsigned char *data,
> + u16 len)
Can fit 1 more tab :)
> +{
> + struct mgmt_cp_set_local_name *mgmt_cp = (void *) data;
> + struct hci_cp_write_local_name hci_cp;
> + struct hci_dev *hdev;
> + struct pending_cmd *cmd;
> + int err;
> +
> + BT_DBG("");
> +
> + if (len != sizeof(*mgmt_cp))
> + return cmd_status(sk, index, MGMT_OP_SET_LOCAL_NAME, EINVAL);
> +
> + hdev = hci_dev_get(index);
> + if (!hdev)
> + return cmd_status(sk, index, MGMT_OP_SET_LOCAL_NAME, ENODEV);
> +
> +
Not needed empty line.
> + hci_dev_lock_bh(hdev);
> +
> + cmd = mgmt_pending_add(sk, MGMT_OP_SET_LOCAL_NAME, index, data, len);
> + if (!cmd) {
> + err = -ENOMEM;
> + goto failed;
> + }
> +
> + memcpy(hci_cp.name, mgmt_cp->name, sizeof(hci_cp.name));
> + err = hci_send_cmd(hdev, HCI_OP_WRITE_LOCAL_NAME, sizeof(hci_cp),
> + &hci_cp);
> + if (err < 0)
> + mgmt_pending_remove(cmd);
> +
> +failed:
> + hci_dev_unlock_bh(hdev);
> + hci_dev_put(hdev);
> +
> + return err;
> +}
> +
> int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
> {
> unsigned char *buf;
> @@ -1353,6 +1393,9 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
> case MGMT_OP_USER_CONFIRM_NEG_REPLY:
> err = user_confirm_reply(sk, index, buf + sizeof(*hdr), len, 0);
> break;
> + case MGMT_OP_SET_LOCAL_NAME:
> + err = set_local_name(sk, index, buf + sizeof(*hdr), len);
> + break;
> default:
> BT_DBG("Unknown op %u", opcode);
> err = cmd_status(sk, index, opcode, 0x01);
> @@ -1649,3 +1692,36 @@ int mgmt_auth_failed(u16 index, bdaddr_t *bdaddr, u8 status)
>
> return mgmt_event(MGMT_EV_AUTH_FAILED, index, &ev, sizeof(ev), NULL);
> }
> +
> +int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status)
> +{
> + struct pending_cmd *cmd;
> + struct mgmt_cp_set_local_name ev;
> + int err;
> +
> + memset(&ev, 0, sizeof(ev));
> + memcpy(ev.name, name, 248);
> +
> + cmd = mgmt_pending_find(MGMT_OP_SET_LOCAL_NAME, index);
> + if (!cmd)
> + goto send_event;
> +
> + if (status) {
> + err = cmd_status(cmd->sk, index, MGMT_OP_SET_LOCAL_NAME, EIO);
> + goto failed;
> + }
> +
> + err = cmd_complete(cmd->sk, index, MGMT_OP_SET_LOCAL_NAME, &ev,
> + sizeof(ev));
> + if (err < 0)
> + goto failed;
> +
> +send_event:
> + err = mgmt_event(MGMT_EV_LOCAL_NAME_CHANGED, index, &ev, sizeof(ev),
> + cmd ? cmd->sk : NULL);
> +
> +failed:
> + if (cmd)
> + mgmt_pending_remove(cmd);
> + return -err;
err is already < 0 here.
> +}
>
And I agree with Anderson Lizardo about 248-249 case.
We should have define for spec name length and use
__u8 name[HCIDEV_NAME_LEN + 1] and comment (at least in docs) why
and that it is guaranteed to be null terminated.
Would also be good to get rid of magic numbers from code (but this is
more generic issue:)
--
Szymon Janc
^ permalink raw reply
* Re: [PATCH v4 2/6] Sim Access Profile Server
From: Johan Hedberg @ 2011-03-16 9:40 UTC (permalink / raw)
To: Waldemar.Rymarkiewicz; +Cc: linux-bluetooth
In-Reply-To: <99B09243E1A5DA4898CDD8B70011144810830AA84A@EXMB04.eu.tieto.com>
Hi Waldek,
On Wed, Mar 16, 2011, Waldemar.Rymarkiewicz@tieto.com wrote:
> >I've pushed the first two patches in this set, but it's gonna
> >take quite a bit longer to properly review the rest (at a
> >quick glance I noticed at least some coding style stuff with
> >over 80-character lines).
>
> Yes, I guess you refer to eg.
>
> case SAP_SET_TRANSPORT_PROTOCOL_REQ:
> if (msg->nparam == 0x01 &&
> msg->param->id == SAP_PARAM_ID_TRANSPORT_PROTOCOL &&
> ntohs(msg->param->len) == SAP_PARAM_ID_TRANSPORT_PROTOCOL_LEN &&
> (*msg->param->val == SAP_TRANSPORT_PROTOCOL_T0 ||
> *msg->param->val == SAP_TRANSPORT_PROTOCOL_T1))
> return 0;
>
> Well, I didn't know how to spit up something like
> "ntohs(msg->param->len) == SAP_PARAM_ID_TRANSPORT_PROTOCOL_LEN &&"
> correctly. Any sugestion?
Use shorter define names (since they really are quite long), move the
checks into separate functions, e.g. validate_set_transport_protocol_req(),
or redo the if-statement to something like:
if (msg->nparam != 0x01)
return -EBADMSG;
if (msg->param->id != SAP_PARAM_ID_TRANSPORT_PROTOCOL)
return -EBADMSG;
if (ntohs(msg->param->len) != SAP_PARAM_ID_TRANSPORT_PROTOCOL_LEN)
return -EBADMSG;
...
return 0;
It becomes much easier for a human to parse it that way since you get to
process the individual conditions clearly one at a time.
Take your pick ;)
Johan
^ permalink raw reply
* RE: [PATCH v4 2/6] Sim Access Profile Server
From: Waldemar.Rymarkiewicz @ 2011-03-16 8:49 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <20110315175408.GC16335@jh-x301>
Sure,
>I've pushed the first two patches in this set, but it's gonna
>take quite a bit longer to properly review the rest (at a
>quick glance I noticed at least some coding style stuff with
>over 80-character lines).
Yes, I guess you refer to eg.
case SAP_SET_TRANSPORT_PROTOCOL_REQ:
if (msg->nparam == 0x01 &&
msg->param->id == SAP_PARAM_ID_TRANSPORT_PROTOCOL &&
ntohs(msg->param->len) == SAP_PARAM_ID_TRANSPORT_PROTOCOL_LEN &&
(*msg->param->val == SAP_TRANSPORT_PROTOCOL_T0 ||
*msg->param->val == SAP_TRANSPORT_PROTOCOL_T1))
return 0;
Well, I didn't know how to spit up something like "ntohs(msg->param->len) == SAP_PARAM_ID_TRANSPORT_PROTOCOL_LEN &&" correctly.
Any sugestion?
/Waldek
^ permalink raw reply
* Re: [RFC v2 0/6] LE advertising cache
From: Ville Tervo @ 2011-03-16 5:53 UTC (permalink / raw)
To: ext Anderson Lizardo; +Cc: ext Andre Guedes, linux-bluetooth
In-Reply-To: <AANLkTikK4vjMgpuBHMLP4-6J2JMMnWqbaDo1-rwFsjEY@mail.gmail.com>
On Tue, Mar 15, 2011 at 10:41:55AM -0400, ext Anderson Lizardo wrote:
> Hi Ville,
>
> On Tue, Mar 15, 2011 at 3:57 AM, Ville Tervo <ville.tervo@nokia.com> wrote:
> > Hi,
> >
> > On Fri, Mar 11, 2011 at 10:32:51AM -0300, ext Andre Guedes wrote:
> >> During a LE connection establishment, the host should be able to infer the
> >> bdaddr type from a given bdaddr.
> >>
> >> To achieve that, during the LE scanning, the host stores the bdaddr and the
> >> bdaddr type gathered from advertising reports. The host keeps a list of
> >> advertising entry (bdaddr and bdaddr_type) for later lookup. This list will
> >> be called Advertising Cache.
> >>
> >> Since the penality to connect to an unreachable device is relatively high,
> >> we must keep only fresh advertising entries on the advertising cache. So,
> >> before each LE scanning the advertising cache is cleared. Also, after the LE
> >> scanning, a timer is set to clear the cache.
> >
> > I tested these pathes with a device which had random address. Connection works
> > which is good.
> >
> > How ever I'm not yet sure if mandatory scanning before every connect is
> > acceptable.
>
> IIRC the connection procedure defined on the spec requires this scan.
> See for instance the general connection establishment procedure at
> page 1715
But it makes "Direct Connection Establishment" impossible.
Is it needed to pass PTS for example?
>
> >
> > I have been playing with idea to derive address type from msb bits of the
> > address. Any ideaѕ what would lose in that way?
>
> How can you differ public address type from a random one in this case?
> I think the MSB bit checking is only for detecting the type of random
> address (static, non-resolvable private, resolvable private)
That's true. Address type needs to be known.
--
Ville
^ permalink raw reply
* Re: [RFC] Auto Connections
From: jaikumar Ganesh @ 2011-03-16 2:57 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: BlueZ development, Ville Tervo
In-Reply-To: <AANLkTimP_XBFy58dMDZa69RQ9UKeZmh++jeuH073doLr@mail.gmail.com>
Hey Claudio:
On Fri, Mar 11, 2011 at 1:30 PM, Claudio Takahasi
<claudio.takahasi@openbossa.org> wrote:
> Hi guys,
>
> It is time to get opinions from some gurus!
>
> We need to implement automatic connections to implement the Profiles.
> At the moment BlueZ supports only dual mode adapters, as consequence
> BlueZ needs to start the LE connection. IMHO, it is better to leave
> the responsibility to the controller, implementing "selective"
> connections will only introduce more code without concrete benefits.
> Configuring the controller to autonomously establish connections seems
> to be the right approach to proceed.
>
> This topic is NOT about StartDiscovery() + CreateDevice.
> StartDiscovery uses active scanning and CreateDevice uses direct
> connection establishment. We need a mechanism to automatically connect
> to "trusted" devices or devices flagged as "AutoConnect".
>
>
> My initial idea is: change the LE server socket to report
> outgoing(host initiated) connections through the server socket.
> Awkward?
> To achieve that we need to manage the LE Create Connection(using
> whitelist) in the kernel, extend the management interface to control
> devices in the whitelist, change the LE Connection Complete Event
> handling to get the Role properly.
> Pros:
> - Controller manage connections
> - Flexible to support connections to "trusted" resolvable address and
> passive scanning
> - Only one "flow" for the connections: LE server socket
> - Maybe we could hide resolvable address from the userspace, mapping
> it directly to public or static random
> Cons:
> - Risky
> - Less control of the connection establishment process
>
>
> Another approach can be to manage the LE connections(using whitelist)
> from the userspace. BDADDR_ANY in the destination field(sockaddr_l2)
> can be used to define LE connections using white list.
> Pros:
> - It will not be necessary to extend the management interface now,
> address can be added directly to the controller's white list
> - Doesn't require changes in the LE server socket
> Cons:
> - BDADDR_ANY in the destination field has no meaning for Basic Rate
> - Needs to expose the resolvable address to the userspace
> - Different "flows" for incoming/outgoing connections
>
>
> Open issue(s):
> * How to handle timeouts when sending LE Create Connection
> * Passive scanning management
>
In general, why do we want let the stack manage auto connections i.e
can you elaborate on "We need to implement automatic connections to
implement the Profiles". Why not let the applications (which sit above
Bluez) handle auto connections ?
Thanks
>
> Cheers,
> Claudio
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [RFC] Auto Connections
From: jaikumar Ganesh @ 2011-03-16 2:56 UTC (permalink / raw)
To: Claudio Takahasi; +Cc: BlueZ development, Ville Tervo
In-Reply-To: <AANLkTimP_XBFy58dMDZa69RQ9UKeZmh++jeuH073doLr@mail.gmail.com>
Hey Claudio:
In general, why do we want let the stack manage auto connections
i.e can you elaborate on "We need to implement automatic connections
to implement the Profiles". Why not let the applications handle auto
connections ?
Thanks
On Fri, Mar 11, 2011 at 1:30 PM, Claudio Takahasi
<claudio.takahasi@openbossa.org> wrote:
> Hi guys,
>
> It is time to get opinions from some gurus!
>
> We need to implement automatic connections to implement the Profiles.
> At the moment BlueZ supports only dual mode adapters, as consequence
> BlueZ needs to start the LE connection. IMHO, it is better to leave
> the responsibility to the controller, implementing "selective"
> connections will only introduce more code without concrete benefits.
> Configuring the controller to autonomously establish connections seems
> to be the right approach to proceed.
>
> This topic is NOT about StartDiscovery() + CreateDevice.
> StartDiscovery uses active scanning and CreateDevice uses direct
> connection establishment. We need a mechanism to automatically connect
> to "trusted" devices or devices flagged as "AutoConnect".
>
>
> My initial idea is: change the LE server socket to report
> outgoing(host initiated) connections through the server socket.
> Awkward?
> To achieve that we need to manage the LE Create Connection(using
> whitelist) in the kernel, extend the management interface to control
> devices in the whitelist, change the LE Connection Complete Event
> handling to get the Role properly.
> Pros:
> - Controller manage connections
> - Flexible to support connections to "trusted" resolvable address and
> passive scanning
> - Only one "flow" for the connections: LE server socket
> - Maybe we could hide resolvable address from the userspace, mapping
> it directly to public or static random
> Cons:
> - Risky
> - Less control of the connection establishment process
>
>
> Another approach can be to manage the LE connections(using whitelist)
> from the userspace. BDADDR_ANY in the destination field(sockaddr_l2)
> can be used to define LE connections using white list.
> Pros:
> - It will not be necessary to extend the management interface now,
> address can be added directly to the controller's white list
> - Doesn't require changes in the LE server socket
> Cons:
> - BDADDR_ANY in the destination field has no meaning for Basic Rate
> - Needs to expose the resolvable address to the userspace
> - Different "flows" for incoming/outgoing connections
>
>
> Open issue(s):
> * How to handle timeouts when sending LE Create Connection
> * Passive scanning management
>
>
> Cheers,
> Claudio
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: Switching between SBC and MPEG audio on headsets
From: Brian Gix @ 2011-03-15 20:50 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Arun Raghavan, linux-bluetooth, Johan Hedberg
In-Reply-To: <AANLkTikGF8NVO5bmZkkKX1pVFMzQD5ip6FRW3bOhiZAe@mail.gmail.com>
Hi Luiz,
On 3/15/2011 1:21 PM, Luiz Augusto von Dentz wrote:
> Hi Brian,
>
> On Tue, Mar 15, 2011 at 4:41 PM, Brian Gix<bgix@codeaurora.org> wrote:
>> On 3/15/2011 12:29 PM, Luiz Augusto von Dentz wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Mar 15, 2011 at 2:01 PM, Johan Hedberg<johan.hedberg@gmail.com>
>>> wrote:
>>>>
>>>> Hi Arun,
>>>>
>>>> On Tue, Mar 15, 2011, Arun Raghavan wrote:
>>>>>
>>>>> I've been trying to set up PulseAudio to be able to switch the A2DP sink
>>>>> dynamically between SBC and MPEG modes. I've got this working now [1],
>>>>> but I
>>>>> did face one problem on the bluez side. When sending a reconfigure
>>>>> request, the
>>>>> request always goes to the SEID that was used previously (=> always to
>>>>> the SBC
>>>>> SEID). Trying to reconfigure for MPEG therefore results the headset
>>>>> returning
>>>>> an error. I'm not very familiar with how this is supposed to work, but
>>>>> the
>>>>> patch following this mail seems to work (I can now switch back and forth
>>>>> between SBC and MPEG modes). It basically forces figuring out what
>>>>> remote SEP
>>>>> to talk to while reconfiguring.
>>>>>
>>>>> Is this the right approach?
>>>>
>>>> I'm not sure about the places where you set the value to NULL, but the
>>>> place in close_cfm which you remove isn't acceptable as such. It was
>>>> originally created to fix the issue reported in this thread:
>>>> http://marc.info/?l=linux-bluetooth&m=129190286303247&w=2
>>>>
>>>> Only a fix which doesn't break the use-case reported there can be
>>>> accepted upstream. FWIW, the commit that introduced the fix is de96fcd8.
>>>
>>> Also the solution should consider a transition and not dropping the
>>> stream completely before configuring the other, otherwise errors may
>>> cause a complete disconnect just to switch between endpoints. Actually
>>> I would suggest configuring both endpoint since the beginning so that
>>> we only need to suspend/resume to switch between them, but I don't
>>> think many headsets would be able to handle this situation.
>>>
>>
>> I think most headsets will not support this because it would require an
>> extra L2CAP connection to be maintained.
>>
>> I don't think there is much of a big deal with closing the first endpoint
>> relationship prior to configuring the next, becuase the AVDTP signaling
>> channel will keep the underlying ACL connection open. Headsets I have worked
>> on in the past would reject a SET_CONFIG if an existing local/remote SEID
>> relationship already existed, not only for the extra resources required to
>> maintain the extra L2CAP channel, but also because the SET_CONFIG is when
>> the hw resources are reserved and configured for the actual PCM data stream.
>>
>> And assuming that the ACL doesn't get dropped, the critical re-setup time is
>> probably most bounded by the underlying hw reconfiguration, and not so much
>> by the few extra small ACL pkts that need to be exchanged.
>
> The problem is that the audio routing is broken if the new stream
> configuration fails, and even if sbc stream is configured latter it
> may not be recovered and some audio maybe lost of rerouted to the
> speakers which I believe is not the best experience for users.
>
> Also we only need another connection in case of AVDTP_Open is sent,
> this sound more a limitation of some implementation than a avdtp/a2dp
> problem, the problem with disconnect is that if the stream setup fails
> the session reference is dropped and the session is released (avdtp
> signalling) so right now we have no means to hold the ACL link if
> nothing else is connected e.g. hfp, we could in theory fix this by
> having a setup which revert to the last configuration if the new one
> fails but I fear this would required a special API to applications be
> able to tells us when this is necessary.
>
I agree that it would be a limitation of the implementation and not the
specification to not allow multiple endpoints to be configured
simultaneously. However many of these headsets are designed on resource
constrained platforms, and it is perfectly allowable for them within the
specification to reject a second endpoint configuration due to
insufficient resources.
And although you could configure two endpoints simultaneously without
opening up both of their streaming L2CAP channels, as soon as you made
the first switch, the second streaming channel would be open. A
suspended streaming channel is still open and consuming resources. The
only way to recover those resources is to issue CLOSE, which then
requires another SET_CONFIG before it could be opened again.
I suppose you could close the channel, and then set up the configuration
again while streaming to the 2nd, so that it is ready to be opened for
the switch-back.
--
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: Switching between SBC and MPEG audio on headsets
From: Brian Gix @ 2011-03-15 20:43 UTC (permalink / raw)
To: Arun Raghavan; +Cc: linux-bluetooth
In-Reply-To: <1300218670.2192.35.camel@snowflake>
Hi Arun,
On 3/15/2011 12:51 PM, Arun Raghavan wrote:
> Hey Brian,
>
[...]
>
> I'm quite unfamiliar with how this works, so please excuse any wild
> inaccuracies in terminology. I believe that I am doing it this way
> already (mostly discovered by trial and error). When I want to
> reconfigure, I do the following:
>
> - Send a BT_STOP_STREAM request, get the expected response
> - Send a BT_CLOSE request, get the expected response
> - Send a BT_OPEN request with the new seid, get the expected response
> - Send a BT_SET_CONFIGURATION request with the new caps, get the
> expected response
> - Send a BT_START_STREAM, wait for the response
> - Start streaming
Well, with full knowledge that I *haven't* examined this part of BlueZ,
I can say that from a protocol perspective, the AVDTP_SET_CONFIG must go
over the air/be acknowledged before the AVDTP_OPEN can be
sent/acknowledged. I haven't checked to see if those step are
obfuscated at all by the BT_OPEN/BT_SET_CONFIGURATION naming convention,
or if it is a simple mapping.
On a compliant device, an Open directly following a Close should return
a NOT_CONFIGURED error. But it may be that a less strict SNK device
might assume the last configuration and overlook the protocol error.
>
> This actually does work here after I apply my patch, although switching
> sample rates doesn't seem to actually happen (probably just something
> broken in my code).
>
> So if I understand this correctly, it should be sufficient to modify my
> patch so we only search for a new remote endpoint if the local endpoint
> changed (or a more specific change limited to if the codec type
> changed)?
>
> Cheers,
> Arun
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: Switching between SBC and MPEG audio on headsets
From: Luiz Augusto von Dentz @ 2011-03-15 20:21 UTC (permalink / raw)
To: Brian Gix; +Cc: Arun Raghavan, linux-bluetooth, Johan Hedberg
In-Reply-To: <4D7FC0D2.5080509@codeaurora.org>
Hi Brian,
On Tue, Mar 15, 2011 at 4:41 PM, Brian Gix <bgix@codeaurora.org> wrote:
> On 3/15/2011 12:29 PM, Luiz Augusto von Dentz wrote:
>>
>> Hi,
>>
>> On Tue, Mar 15, 2011 at 2:01 PM, Johan Hedberg<johan.hedberg@gmail.com>
>> wrote:
>>>
>>> Hi Arun,
>>>
>>> On Tue, Mar 15, 2011, Arun Raghavan wrote:
>>>>
>>>> I've been trying to set up PulseAudio to be able to switch the A2DP sink
>>>> dynamically between SBC and MPEG modes. I've got this working now [1],
>>>> but I
>>>> did face one problem on the bluez side. When sending a reconfigure
>>>> request, the
>>>> request always goes to the SEID that was used previously (=> always to
>>>> the SBC
>>>> SEID). Trying to reconfigure for MPEG therefore results the headset
>>>> returning
>>>> an error. I'm not very familiar with how this is supposed to work, but
>>>> the
>>>> patch following this mail seems to work (I can now switch back and forth
>>>> between SBC and MPEG modes). It basically forces figuring out what
>>>> remote SEP
>>>> to talk to while reconfiguring.
>>>>
>>>> Is this the right approach?
>>>
>>> I'm not sure about the places where you set the value to NULL, but the
>>> place in close_cfm which you remove isn't acceptable as such. It was
>>> originally created to fix the issue reported in this thread:
>>> http://marc.info/?l=linux-bluetooth&m=129190286303247&w=2
>>>
>>> Only a fix which doesn't break the use-case reported there can be
>>> accepted upstream. FWIW, the commit that introduced the fix is de96fcd8.
>>
>> Also the solution should consider a transition and not dropping the
>> stream completely before configuring the other, otherwise errors may
>> cause a complete disconnect just to switch between endpoints. Actually
>> I would suggest configuring both endpoint since the beginning so that
>> we only need to suspend/resume to switch between them, but I don't
>> think many headsets would be able to handle this situation.
>>
>
> I think most headsets will not support this because it would require an
> extra L2CAP connection to be maintained.
>
> I don't think there is much of a big deal with closing the first endpoint
> relationship prior to configuring the next, becuase the AVDTP signaling
> channel will keep the underlying ACL connection open. Headsets I have worked
> on in the past would reject a SET_CONFIG if an existing local/remote SEID
> relationship already existed, not only for the extra resources required to
> maintain the extra L2CAP channel, but also because the SET_CONFIG is when
> the hw resources are reserved and configured for the actual PCM data stream.
>
> And assuming that the ACL doesn't get dropped, the critical re-setup time is
> probably most bounded by the underlying hw reconfiguration, and not so much
> by the few extra small ACL pkts that need to be exchanged.
The problem is that the audio routing is broken if the new stream
configuration fails, and even if sbc stream is configured latter it
may not be recovered and some audio maybe lost of rerouted to the
speakers which I believe is not the best experience for users.
Also we only need another connection in case of AVDTP_Open is sent,
this sound more a limitation of some implementation than a avdtp/a2dp
problem, the problem with disconnect is that if the stream setup fails
the session reference is dropped and the session is released (avdtp
signalling) so right now we have no means to hold the ACL link if
nothing else is connected e.g. hfp, we could in theory fix this by
having a setup which revert to the last configuration if the new one
fails but I fear this would required a special API to applications be
able to tells us when this is necessary.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH 3/3] Bluetooth: mgmt: Add support for setting the local name
From: Anderson Lizardo @ 2011-03-15 19:57 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1300215293-30294-3-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
On Tue, Mar 15, 2011 at 2:54 PM, <johan.hedberg@gmail.com> wrote:
> From: Johan Hedberg <johan.hedberg@nokia.com>
>
> This patch adds a new set_local_name management command as well as a
> local_name_changed management event. With these user space can both
> change the local name as well as monitor changes to it by others.
>
> The management messages reserve 249 bytes for the name instead of 248
> (like in the HCI spec) so that there is always a guarantee that it is
> nul-terminated. That way it can safely be passed onto string
> manipulation functions.
I wonder if it is "future-proof" to use 248 and 249 along the code
without using some #define for the the spec defined value. Someone may
look at the code later and think there is a bug, even though it is
clear from the commit message.
My two cents,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: Switching between SBC and MPEG audio on headsets
From: Arun Raghavan @ 2011-03-15 19:51 UTC (permalink / raw)
To: Brian Gix; +Cc: linux-bluetooth
In-Reply-To: <4D7F9237.2080206@codeaurora.org>
Hey Brian,
On Tue, 2011-03-15 at 09:22 -0700, Brian Gix wrote:
[...]
> No this is the incorrect approach.
Thanks (to you and Johan) for the detailed explanation! :)
> Both SRC and SNK devices have a variety of endpoints that have
> capabilities describing what that endpoint is capable of, the most
> important of which is the Codec Capability. Each endpoint is allowed a
> single capability of a particular type, which means that an endpoint
> that supports SBC should not attempt to use a different codec (for MPEG
> in this case). Any device that attempts to wedge both codecs into a
> single endpoint would most definitely be non-compliant.
>
> The SUSPEND/RECONFIGURE/START procedure will not work to switch between
> SBC and MPEG for this reason. The purpose for that procedure is to
> allow quick changes within the bounds of the current (remote and local)
> endpoint capabilities. To quickly switch between a 44.1KHz and 48KHz
> sampling for instance.
>
> The correct procedure to switch between SBC and MPEG would be to close
> the existing media channel (but NOT the AVDTP signaling channel) and
> reset up a new endpoint. If you know you will be doing this, you can
> save time by caching all of the remote devices endpoints the first time
> you do an AVDTP_DISCOVER, so that you have all the remote endpoint info
> you need to make the switch (and even know if the switch will work). You
> also should have all supported local endpoints separated into their own
> endpoints of course.
>
> The to switch, you will need to:
>
> <halt streaming>
>
> AVDTP_CLOSE (on AVDTP signaling channel)
>
> L2CAP_CLOSE (close the media L2CAP channel for SBC)
>
> <<<< You should Rx AVDTP_CLOSE_CFM
>
> AVDTP_SET_CONFIG (on AVDTP signaling channel,
> specify a local and remote endpoint that are compatible)
>
> <<<<< You should Rx SET_CONFIG_CFM
>
> AVDTP_OPEN (on AVDTP signaling channel)
>
> L2CAP_OPEN (open new media L2CAP channel for MPEG)
>
> <<<<< You should Rx AVDTP_OPEN_CFM
>
> AVDTP_START (on AVDTP signaling channel)
>
> <<<<< You should Rx AVDTP_START_CFM
>
> <start streaming>
I'm quite unfamiliar with how this works, so please excuse any wild
inaccuracies in terminology. I believe that I am doing it this way
already (mostly discovered by trial and error). When I want to
reconfigure, I do the following:
- Send a BT_STOP_STREAM request, get the expected response
- Send a BT_CLOSE request, get the expected response
- Send a BT_OPEN request with the new seid, get the expected response
- Send a BT_SET_CONFIGURATION request with the new caps, get the
expected response
- Send a BT_START_STREAM, wait for the response
- Start streaming
This actually does work here after I apply my patch, although switching
sample rates doesn't seem to actually happen (probably just something
broken in my code).
So if I understand this correctly, it should be sufficient to modify my
patch so we only search for a new remote endpoint if the local endpoint
changed (or a more specific change limited to if the codec type
changed)?
Cheers,
Arun
^ permalink raw reply
* Re: Switching between SBC and MPEG audio on headsets
From: Brian Gix @ 2011-03-15 19:41 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Arun Raghavan, linux-bluetooth, Johan Hedberg
In-Reply-To: <AANLkTimF0Ux=-uQiWaWH14r3_YAuyp2o=QrmyeLkWWeT@mail.gmail.com>
On 3/15/2011 12:29 PM, Luiz Augusto von Dentz wrote:
> Hi,
>
> On Tue, Mar 15, 2011 at 2:01 PM, Johan Hedberg<johan.hedberg@gmail.com> wrote:
>> Hi Arun,
>>
>> On Tue, Mar 15, 2011, Arun Raghavan wrote:
>>> I've been trying to set up PulseAudio to be able to switch the A2DP sink
>>> dynamically between SBC and MPEG modes. I've got this working now [1], but I
>>> did face one problem on the bluez side. When sending a reconfigure request, the
>>> request always goes to the SEID that was used previously (=> always to the SBC
>>> SEID). Trying to reconfigure for MPEG therefore results the headset returning
>>> an error. I'm not very familiar with how this is supposed to work, but the
>>> patch following this mail seems to work (I can now switch back and forth
>>> between SBC and MPEG modes). It basically forces figuring out what remote SEP
>>> to talk to while reconfiguring.
>>>
>>> Is this the right approach?
>>
>> I'm not sure about the places where you set the value to NULL, but the
>> place in close_cfm which you remove isn't acceptable as such. It was
>> originally created to fix the issue reported in this thread:
>> http://marc.info/?l=linux-bluetooth&m=129190286303247&w=2
>>
>> Only a fix which doesn't break the use-case reported there can be
>> accepted upstream. FWIW, the commit that introduced the fix is de96fcd8.
>
> Also the solution should consider a transition and not dropping the
> stream completely before configuring the other, otherwise errors may
> cause a complete disconnect just to switch between endpoints. Actually
> I would suggest configuring both endpoint since the beginning so that
> we only need to suspend/resume to switch between them, but I don't
> think many headsets would be able to handle this situation.
>
I think most headsets will not support this because it would require an
extra L2CAP connection to be maintained.
I don't think there is much of a big deal with closing the first
endpoint relationship prior to configuring the next, becuase the AVDTP
signaling channel will keep the underlying ACL connection open. Headsets
I have worked on in the past would reject a SET_CONFIG if an existing
local/remote SEID relationship already existed, not only for the extra
resources required to maintain the extra L2CAP channel, but also because
the SET_CONFIG is when the hw resources are reserved and configured for
the actual PCM data stream.
And assuming that the ACL doesn't get dropped, the critical re-setup
time is probably most bounded by the underlying hw reconfiguration, and
not so much by the few extra small ACL pkts that need to be exchanged.
--
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: Switching between SBC and MPEG audio on headsets
From: Luiz Augusto von Dentz @ 2011-03-15 19:29 UTC (permalink / raw)
To: Arun Raghavan, linux-bluetooth; +Cc: Johan Hedberg
In-Reply-To: <20110315170124.GA15712@jh-x301>
Hi,
On Tue, Mar 15, 2011 at 2:01 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Arun,
>
> On Tue, Mar 15, 2011, Arun Raghavan wrote:
>> I've been trying to set up PulseAudio to be able to switch the A2DP sink
>> dynamically between SBC and MPEG modes. I've got this working now [1], but I
>> did face one problem on the bluez side. When sending a reconfigure request, the
>> request always goes to the SEID that was used previously (=> always to the SBC
>> SEID). Trying to reconfigure for MPEG therefore results the headset returning
>> an error. I'm not very familiar with how this is supposed to work, but the
>> patch following this mail seems to work (I can now switch back and forth
>> between SBC and MPEG modes). It basically forces figuring out what remote SEP
>> to talk to while reconfiguring.
>>
>> Is this the right approach?
>
> I'm not sure about the places where you set the value to NULL, but the
> place in close_cfm which you remove isn't acceptable as such. It was
> originally created to fix the issue reported in this thread:
> http://marc.info/?l=linux-bluetooth&m=129190286303247&w=2
>
> Only a fix which doesn't break the use-case reported there can be
> accepted upstream. FWIW, the commit that introduced the fix is de96fcd8.
Also the solution should consider a transition and not dropping the
stream completely before configuring the other, otherwise errors may
cause a complete disconnect just to switch between endpoints. Actually
I would suggest configuring both endpoint since the beginning so that
we only need to suspend/resume to switch between them, but I don't
think many headsets would be able to handle this situation.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [bluetooth-next 04/15] Bluetooth: Add support for using the crypto subsystem
From: Brian Gix @ 2011-03-15 19:12 UTC (permalink / raw)
To: Anderson Briglia; +Cc: Vinicius Costa Gomes, linux-bluetooth
In-Reply-To: <AANLkTinQh-oM+Dn-j8Rt8s99edozbXFjC-MUwiOLd_y1@mail.gmail.com>
On 3/15/2011 12:03 PM, Anderson Briglia wrote:
> Hi all,
>
> On Wed, Mar 9, 2011 at 6:52 PM, Vinicius Costa Gomes
> <vinicius.gomes@openbossa.org> wrote:
>> On 14:45 Thu 03 Mar, Vinicius Costa Gomes wrote:
>>> Hi Marcel,
>>>
>>> On 14:28 Mon 28 Feb, Gustavo F. Padovan wrote:
>>>> Hi Vinicius,
>>>>
>>>> * Vinicius Gomes<vinicius.gomes@openbossa.org> [2011-02-27 21:49:39 -0300]:
>>>>
>>>>> Hi Gustavo,
>>>>>
>>>>> On Sun, Feb 27, 2011 at 5:20 PM, Gustavo F. Padovan
>>>>> <padovan@profusion.mobi> wrote:
>>>>>> Hi Vinicius,
>>>>>>
>>>>>> * Vinicius Costa Gomes<vinicius.gomes@openbossa.org> [2011-02-21 14:23:51 -0300]:
>>>>>>
[...]
>>>
>>> Any ideas?
>>
>> Still no ideas?
>>
>
> We at INdT have some patches based on top of this series. What is the
> maintainer's opinion about this series? I'm asking it because should
> be great to know if we are doing the right stuff or we need to change
> our approach. The number of features and patches that have this series
> as dependency is growing.
I am also developing patches off of this series and have signed off on
their baseline functionality.
> Regards,
>
> Anderson Briglia
--
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: [bluetooth-next 04/15] Bluetooth: Add support for using the crypto subsystem
From: Anderson Briglia @ 2011-03-15 19:03 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <20110309225246.GA3009@piper>
Hi all,
On Wed, Mar 9, 2011 at 6:52 PM, Vinicius Costa Gomes
<vinicius.gomes@openbossa.org> wrote:
> On 14:45 Thu 03 Mar, Vinicius Costa Gomes wrote:
>> Hi Marcel,
>>
>> On 14:28 Mon 28 Feb, Gustavo F. Padovan wrote:
>> > Hi Vinicius,
>> >
>> > * Vinicius Gomes <vinicius.gomes@openbossa.org> [2011-02-27 21:49:39 -0300]:
>> >
>> > > Hi Gustavo,
>> > >
>> > > On Sun, Feb 27, 2011 at 5:20 PM, Gustavo F. Padovan
>> > > <padovan@profusion.mobi> wrote:
>> > > > Hi Vinicius,
>> > > >
>> > > > * Vinicius Costa Gomes <vinicius.gomes@openbossa.org> [2011-02-21 14:23:51 -0300]:
>> > > >
>> > > >> This will allow using the crypto subsystem for encrypting data. As SMP
>> > > >> (Security Manager Protocol) is implemented almost entirely on the host
>> > > >> side and the crypto module already implements the needed methods
>> > > >> (AES-128), it makes sense to use it.
>> > > >>
>> > > >> This patch also adds a new Kconfig option to toggle the SMP support.
>> > > >>
>> > > >> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
>> > > >> Signed-off-by: Anderson Briglia <anderson.briglia@openbossa.org>
>> > > >> ---
>> > > >> include/net/bluetooth/hci_core.h | 2 ++
>> > > >> net/bluetooth/Kconfig | 6 ++++++
>> > > >> net/bluetooth/hci_core.c | 22 ++++++++++++++++++++++
>> > > >> net/bluetooth/smp.c | 17 +++++++++++++++--
>> > > >> 4 files changed, 45 insertions(+), 2 deletions(-)
>> > > >>
>> > > >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> > > >> index d5d8454..e8dbde8 100644
>> > > >> --- a/include/net/bluetooth/hci_core.h
>> > > >> +++ b/include/net/bluetooth/hci_core.h
>> > > >> @@ -161,6 +161,8 @@ struct hci_dev {
>> > > >>
>> > > >> __u16 init_last_cmd;
>> > > >>
>> > > >> + struct crypto_blkcipher *tfm;
>> > > >> +
>> > > >> struct inquiry_cache inq_cache;
>> > > >> struct hci_conn_hash conn_hash;
>> > > >> struct list_head blacklist;
>> > > >> diff --git a/net/bluetooth/Kconfig b/net/bluetooth/Kconfig
>> > > >> index c6f9c2f..e9f40af 100644
>> > > >> --- a/net/bluetooth/Kconfig
>> > > >> +++ b/net/bluetooth/Kconfig
>> > > >> @@ -22,6 +22,7 @@ menuconfig BT
>> > > >> BNEP Module (Bluetooth Network Encapsulation Protocol)
>> > > >> CMTP Module (CAPI Message Transport Protocol)
>> > > >> HIDP Module (Human Interface Device Protocol)
>> > > >> + SMP Module (Security Manager Protocol)
>> > > >>
>> > > >> Say Y here to compile Bluetooth support into the kernel or say M to
>> > > >> compile it as module (bluetooth).
>> > > >> @@ -35,11 +36,16 @@ config BT_L2CAP
>> > > >> bool "L2CAP protocol support"
>> > > >> depends on BT
>> > > >> select CRC16
>> > > >> + select CRYPTO_BLKCIPHER
>> > > >> + select CRYPTO_AES
>> > > >> help
>> > > >> L2CAP (Logical Link Control and Adaptation Protocol) provides
>> > > >> connection oriented and connection-less data transport. L2CAP
>> > > >> support is required for most Bluetooth applications.
>> > > >>
>> > > >> + Also included is support for SMP (Security Manager Protocol) which
>> > > >> + is the security layer on top of LE (Low Energy) links.
>> > > >> +
>> > > >> config BT_SCO
>> > > >> bool "SCO links support"
>> > > >> depends on BT
>> > > >> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> > > >> index b372fb8..ff67843 100644
>> > > >> --- a/net/bluetooth/hci_core.c
>> > > >> +++ b/net/bluetooth/hci_core.c
>> > > >> @@ -42,6 +42,7 @@
>> > > >> #include <linux/notifier.h>
>> > > >> #include <linux/rfkill.h>
>> > > >> #include <linux/timer.h>
>> > > >> +#include <linux/crypto.h>
>> > > >> #include <net/sock.h>
>> > > >>
>> > > >> #include <asm/system.h>
>> > > >> @@ -60,6 +61,8 @@ static void hci_notify(struct hci_dev *hdev, int event);
>> > > >>
>> > > >> static DEFINE_RWLOCK(hci_task_lock);
>> > > >>
>> > > >> +static int enable_smp;
>> > > >> +
>> > > >> /* HCI device list */
>> > > >> LIST_HEAD(hci_dev_list);
>> > > >> DEFINE_RWLOCK(hci_dev_list_lock);
>> > > >> @@ -1077,6 +1080,14 @@ static void hci_cmd_timer(unsigned long arg)
>> > > >> tasklet_schedule(&hdev->cmd_task);
>> > > >> }
>> > > >>
>> > > >> +static struct crypto_blkcipher *alloc_cypher(void)
>> > > >> +{
>> > > >> + if (enable_smp)
>> > > >> + return crypto_alloc_blkcipher("ecb(aes)", 0, CRYPTO_ALG_ASYNC);
>> > > >> +
>> > > >> + return ERR_PTR(-ENOTSUPP);
>> > > >> +}
>> > > >> +
>> > > >> /* Register HCI device */
>> > > >> int hci_register_dev(struct hci_dev *hdev)
>> > > >> {
>> > > >> @@ -1155,6 +1166,11 @@ int hci_register_dev(struct hci_dev *hdev)
>> > > >> if (!hdev->workqueue)
>> > > >> goto nomem;
>> > > >>
>> > > >> + hdev->tfm = alloc_cypher();
>> > > >> + if (IS_ERR(hdev->tfm))
>> > > >> + BT_INFO("Failed to load transform for ecb(aes): %ld",
>> > > >> + PTR_ERR(hdev->tfm));
>> > > >> +
>> > > >> hci_register_sysfs(hdev);
>> > > >>
>> > > >> hdev->rfkill = rfkill_alloc(hdev->name, &hdev->dev,
>> > > >> @@ -1203,6 +1219,9 @@ int hci_unregister_dev(struct hci_dev *hdev)
>> > > >> !test_bit(HCI_SETUP, &hdev->flags))
>> > > >> mgmt_index_removed(hdev->id);
>> > > >>
>> > > >> + if (!IS_ERR(hdev->tfm))
>> > > >> + crypto_free_blkcipher(hdev->tfm);
>> > > >> +
>> > > >> hci_notify(hdev, HCI_DEV_UNREG);
>> > > >>
>> > > >> if (hdev->rfkill) {
>> > > >> @@ -2037,3 +2056,6 @@ static void hci_cmd_task(unsigned long arg)
>> > > >> }
>> > > >> }
>> > > >> }
>> > > >> +
>> > > >> +module_param(enable_smp, bool, 0644);
>> > > >> +MODULE_PARM_DESC(enable_smp, "Enable SMP support (LE only)");
>> > > >
>> > > > This all should be obviously inside smp.c
>> > >
>> > > I have a couple of points against it:
>> > >
>> > > 1. it's only used for one purpose, it says whether to try or not to
>> > > allocate the block cypher, which is done during the adapter
>> > > registration;
>> > >
>> > > 2. If the current way is ok, it would mean that I would need to export
>> > > another method from smp.c, that was something that I tried to
>> > > minimize;
>> > >
>> > > 3. and my weakest argument, seems that there are other similar uses,
>> > > for example enable_mgmt.
>> >
>> > A similar example here is enable_ertm inside l2cap_core.c. It's a L2CAP
>> > related option and should reside inside L2CAP code.
>> >
>> > >
>> > > But if you think the code will be clearer moving that to smp.c, will
>> > > be glad to change.
>> >
>> > I don't see the point on have it on hci code. SMP and Block cypher has
>> > not much to do with HCI. I prefer it on smp.c
>>
>> Gustavo and I were talking on IRC and couldn't come to a solution, so
>> we ask you for some input.
>>
>> So, just to give a little more context, the problem is that the crypto
>> functions used in SMP depend on the allocation of a block cypher "transform",
>> this allocation must happen during user context.
>>
>> The current solution is that the allocation is done in hci_core.c during the
>> adapter registration, but only when the enable_smp module parameter is enabled.
>> When the parameter is disabled the allocation method just returns an invalid
>> pointer and everything happens the same as if the allocation has failed.
>>
>> Gustavo has a preference for an ondemand aproach, for example, using a
>> workqueue for doing the allocation, and trying the allocation just when SMP is
>> going to be used, e.g. when we receive the first SMP message or when some
>> security level is activated for that socket.
>>
>> Any ideas?
>
> Still no ideas?
>
We at INdT have some patches based on top of this series. What is the
maintainer's opinion about this series? I'm asking it because should
be great to know if we are doing the right stuff or we need to change
our approach. The number of features and patches that have this series
as dependency is growing.
Regards,
Anderson Briglia
>>
>> >
>> > --
>> > Gustavo F. Padovan
>> > http://profusion.mobi
>>
>>
>> Cheers,
>> --
>> Vinicius
>
> --
> Vinicius
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
INdT - Instituto Nokia de tecnologia
+55 2126 1122
http://techblog.briglia.net
^ permalink raw reply
* [PATCH 3/3] Bluetooth: mgmt: Add support for setting the local name
From: johan.hedberg @ 2011-03-15 18:54 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1300215293-30294-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@nokia.com>
This patch adds a new set_local_name management command as well as a
local_name_changed management event. With these user space can both
change the local name as well as monitor changes to it by others.
The management messages reserve 249 bytes for the name instead of 248
(like in the HCI spec) so that there is always a guarantee that it is
nul-terminated. That way it can safely be passed onto string
manipulation functions.
Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
include/net/bluetooth/hci_core.h | 1 +
include/net/bluetooth/mgmt.h | 10 +++++
net/bluetooth/hci_event.c | 9 +++-
net/bluetooth/mgmt.c | 76 ++++++++++++++++++++++++++++++++++++++
4 files changed, 93 insertions(+), 3 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 441dadb..c77c082 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -767,6 +767,7 @@ int mgmt_user_confirm_reply_complete(u16 index, bdaddr_t *bdaddr, u8 status);
int mgmt_user_confirm_neg_reply_complete(u16 index, bdaddr_t *bdaddr,
u8 status);
int mgmt_auth_failed(u16 index, bdaddr_t *bdaddr, u8 status);
+int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status);
/* HCI info for socket */
#define hci_pi(sk) ((struct hci_pinfo *) sk)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 4deeea1..f7d4df3 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -168,6 +168,11 @@ struct mgmt_rp_user_confirm_reply {
#define MGMT_OP_USER_CONFIRM_NEG_REPLY 0x0016
+#define MGMT_OP_SET_LOCAL_NAME 0x0017
+struct mgmt_cp_set_local_name {
+ __u8 name[249];
+} __packed;
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
@@ -235,3 +240,8 @@ struct mgmt_ev_auth_failed {
bdaddr_t bdaddr;
__u8 status;
} __packed;
+
+#define MGMT_EV_LOCAL_NAME_CHANGED 0x0011
+struct mgmt_ev_local_name_changed {
+ __u8 name[249];
+} __packed;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3fbfa50..e7f7cb0 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -193,13 +193,16 @@ static void hci_cc_write_local_name(struct hci_dev *hdev, struct sk_buff *skb)
BT_DBG("%s status 0x%x", hdev->name, status);
- if (status)
- return;
-
sent = hci_sent_cmd_data(hdev, HCI_OP_WRITE_LOCAL_NAME);
if (!sent)
return;
+ if (test_bit(HCI_MGMT, &hdev->flags))
+ mgmt_set_local_name_complete(hdev->id, sent, status);
+
+ if (status)
+ return;
+
memcpy(hdev->dev_name, sent, 248);
}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 67529c8..6e84ab2 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1258,6 +1258,46 @@ failed:
return err;
}
+static int set_local_name(struct sock *sk, u16 index, unsigned char *data,
+ u16 len)
+{
+ struct mgmt_cp_set_local_name *mgmt_cp = (void *) data;
+ struct hci_cp_write_local_name hci_cp;
+ struct hci_dev *hdev;
+ struct pending_cmd *cmd;
+ int err;
+
+ BT_DBG("");
+
+ if (len != sizeof(*mgmt_cp))
+ return cmd_status(sk, index, MGMT_OP_SET_LOCAL_NAME, EINVAL);
+
+ hdev = hci_dev_get(index);
+ if (!hdev)
+ return cmd_status(sk, index, MGMT_OP_SET_LOCAL_NAME, ENODEV);
+
+
+ hci_dev_lock_bh(hdev);
+
+ cmd = mgmt_pending_add(sk, MGMT_OP_SET_LOCAL_NAME, index, data, len);
+ if (!cmd) {
+ err = -ENOMEM;
+ goto failed;
+ }
+
+ memcpy(hci_cp.name, mgmt_cp->name, sizeof(hci_cp.name));
+ err = hci_send_cmd(hdev, HCI_OP_WRITE_LOCAL_NAME, sizeof(hci_cp),
+ &hci_cp);
+ if (err < 0)
+ mgmt_pending_remove(cmd);
+
+failed:
+ hci_dev_unlock_bh(hdev);
+ hci_dev_put(hdev);
+
+ return err;
+}
+
int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
{
unsigned char *buf;
@@ -1353,6 +1393,9 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
case MGMT_OP_USER_CONFIRM_NEG_REPLY:
err = user_confirm_reply(sk, index, buf + sizeof(*hdr), len, 0);
break;
+ case MGMT_OP_SET_LOCAL_NAME:
+ err = set_local_name(sk, index, buf + sizeof(*hdr), len);
+ break;
default:
BT_DBG("Unknown op %u", opcode);
err = cmd_status(sk, index, opcode, 0x01);
@@ -1649,3 +1692,36 @@ int mgmt_auth_failed(u16 index, bdaddr_t *bdaddr, u8 status)
return mgmt_event(MGMT_EV_AUTH_FAILED, index, &ev, sizeof(ev), NULL);
}
+
+int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status)
+{
+ struct pending_cmd *cmd;
+ struct mgmt_cp_set_local_name ev;
+ int err;
+
+ memset(&ev, 0, sizeof(ev));
+ memcpy(ev.name, name, 248);
+
+ cmd = mgmt_pending_find(MGMT_OP_SET_LOCAL_NAME, index);
+ if (!cmd)
+ goto send_event;
+
+ if (status) {
+ err = cmd_status(cmd->sk, index, MGMT_OP_SET_LOCAL_NAME, EIO);
+ goto failed;
+ }
+
+ err = cmd_complete(cmd->sk, index, MGMT_OP_SET_LOCAL_NAME, &ev,
+ sizeof(ev));
+ if (err < 0)
+ goto failed;
+
+send_event:
+ err = mgmt_event(MGMT_EV_LOCAL_NAME_CHANGED, index, &ev, sizeof(ev),
+ cmd ? cmd->sk : NULL);
+
+failed:
+ if (cmd)
+ mgmt_pending_remove(cmd);
+ return -err;
+}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 2/3] Bluetooth: mgmt: Add local name information to read_info reply
From: johan.hedberg @ 2011-03-15 18:54 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1300215293-30294-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@nokia.com>
This patch adds the name of the adapter to the reply of the read_info
management command.
Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
include/net/bluetooth/mgmt.h | 1 +
net/bluetooth/mgmt.c | 4 ++++
2 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 5fabfa8..4deeea1 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -55,6 +55,7 @@ struct mgmt_rp_read_info {
__u16 manufacturer;
__u8 hci_ver;
__u16 hci_rev;
+ __u8 name[249];
} __packed;
struct mgmt_mode {
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 4476d8e..67529c8 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -183,6 +183,8 @@ static int read_controller_info(struct sock *sk, u16 index)
set_bit(HCI_MGMT, &hdev->flags);
+ memset(&rp, 0, sizeof(rp));
+
rp.type = hdev->dev_type;
rp.powered = test_bit(HCI_UP, &hdev->flags);
@@ -204,6 +206,8 @@ static int read_controller_info(struct sock *sk, u16 index)
rp.hci_ver = hdev->hci_ver;
put_unaligned_le16(hdev->hci_rev, &rp.hci_rev);
+ memcpy(rp.name, hdev->dev_name, sizeof(hdev->dev_name));
+
hci_dev_unlock_bh(hdev);
hci_dev_put(hdev);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 1/3] Bluetooth: Fix missing hci_dev_lock_bh in user_confirm_reply
From: johan.hedberg @ 2011-03-15 18:54 UTC (permalink / raw)
To: linux-bluetooth
From: Johan Hedberg <johan.hedberg@nokia.com>
The code was correctly calling _unlock at the end of the function but
there was no actual _lock call anywhere.
Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
net/bluetooth/mgmt.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 0054c74..4476d8e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1230,6 +1230,8 @@ static int user_confirm_reply(struct sock *sk, u16 index, unsigned char *data,
if (!hdev)
return cmd_status(sk, index, mgmt_op, ENODEV);
+ hci_dev_lock_bh(hdev);
+
if (!test_bit(HCI_UP, &hdev->flags)) {
err = cmd_status(sk, index, mgmt_op, ENETDOWN);
goto failed;
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v4 2/6] Sim Access Profile Server
From: Johan Hedberg @ 2011-03-15 17:54 UTC (permalink / raw)
To: Waldemar Rymarkiewicz; +Cc: linux-bluetooth
In-Reply-To: <1300207401-23438-3-git-send-email-waldemar.rymarkiewicz@tieto.com>
Hi Waldek,
On Tue, Mar 15, 2011, Waldemar Rymarkiewicz wrote:
> Add a Sim Access Server to the SAP plugin and a framework for the dummy
> sap driver as well.
>
> * add the server register and unregister rutines
> * add server listening socket setup
> * add SAP DBus API
> * add prototypes for SAP protocol implementation
> * add skeleton of dummy SIM driver
> ---
> Makefile.am | 3 +-
> sap/sap-dummy.c | 85 ++++++++
> sap/sap.h | 186 +++++++++++++++++
> sap/server.c | 612 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 878 insertions(+), 8 deletions(-)
> create mode 100644 sap/sap-dummy.c
> create mode 100644 sap/sap.h
I've pushed the first two patches in this set, but it's gonna take quite
a bit longer to properly review the rest (at a quick glance I noticed at
least some coding style stuff with over 80-character lines). So the rest
will have to wait until after the next BlueZ release (which will
hopefully happen still today).
Johan
^ permalink raw reply
* Re: [PATCH v4 1/2] Use new UUID functions in GATT
From: Johan Hedberg @ 2011-03-15 17:39 UTC (permalink / raw)
To: Elvis, Pf; +Cc: linux-bluetooth
In-Reply-To: <1300194304-29449-1-git-send-email-epx@signove.com>
Hi Elvis,
On Tue, Mar 15, 2011, Elvis Pf??tzenreuter wrote:
> This patch puts the new UUID functions into use for GATT-related
> code, and adds some convenience functions to ATT API (att.h).
> Example GATT server is also changed.
> ---
> attrib/att.c | 47 +++++++++++--------------
> attrib/att.h | 69 +++++++++++++++++++++++++++++++++----
> attrib/client.c | 26 ++++++--------
> attrib/example.c | 92 ++++++++++++++++++++++++++-----------------------
> attrib/gatt.c | 68 +++++++++++++++++++------------------
> attrib/gatt.h | 4 +-
> attrib/gattrib.c | 3 +-
> attrib/gatttool.c | 18 ++++-----
> attrib/interactive.c | 22 +++++------
> attrib/utils.c | 1 +
> src/adapter.c | 1 +
> src/attrib-server.c | 93 +++++++++++++++++++++++++------------------------
> src/attrib-server.h | 4 +-
> src/device.c | 1 +
> src/main.c | 1 +
> 15 files changed, 250 insertions(+), 200 deletions(-)
Both patches have been pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 1/3] Move telephony_last_dialed_number_req
From: Johan Hedberg @ 2011-03-15 17:36 UTC (permalink / raw)
To: Dmitriy Paliy; +Cc: linux-bluetooth
In-Reply-To: <1300193754-5748-1-git-send-email-dmitriy.paliy@nokia.com>
Hi Dmitriy,
On Tue, Mar 15, 2011, Dmitriy Paliy wrote:
> telephony_last_dialed_number_req function moved to be after
> send_method_call. It is used in successive commit.
> ---
> audio/telephony-maemo6.c | 24 ++++++++++++------------
> 1 files changed, 12 insertions(+), 12 deletions(-)
All three patches have been pushed upstream. Thanks.
Johan
^ 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