2011-03-16 11:25:08

by Bruna Moreira

[permalink] [raw]
Subject: [PATCH 1/5] TODO: remove 'fix MTU exchange' task

---
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



2011-03-18 09:24:11

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCHv2 1/5] TODO: remove 'fix MTU exchange' task

Hi Bruna,

On Thu, Mar 17, 2011, Bruna Moreira wrote:
> ---
> TODO | 5 -----
> 1 files changed, 0 insertions(+), 5 deletions(-)

All five patches have now been pushed upstream. Thanks.

Johan

2011-03-17 14:55:20

by Bruna Moreira

[permalink] [raw]
Subject: [PATCHv2 5/5] Add Exchange MTU in interactive gatttool

---
attrib/interactive.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/attrib/interactive.c b/attrib/interactive.c
index a9157e7..3fafb1e 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -362,6 +362,7 @@ static void cmd_disconnect(int argcp, char **argvp)

g_attrib_unref(attrib);
attrib = NULL;
+ opt_mtu = 0;

g_io_channel_shutdown(iochannel, FALSE, NULL);
g_io_channel_unref(iochannel);
@@ -654,6 +655,65 @@ static void cmd_sec_level(int argcp, char **argvp)
}
}

+static void exchange_mtu_cb(guint8 status, const guint8 *pdu, guint16 plen,
+ gpointer user_data)
+{
+ uint16_t mtu;
+
+ if (status != 0) {
+ printf("Exchange MTU Request failed: %s\n",
+ att_ecode2str(status));
+ return;
+ }
+
+ if (!dec_mtu_resp(pdu, plen, &mtu)) {
+ printf("Protocol error\n");
+ return;
+ }
+
+ mtu = MIN(mtu, opt_mtu);
+ /* Set new value for MTU in client */
+ if (g_attrib_set_mtu(attrib, mtu))
+ printf("MTU was exchanged successfully: %d\n", mtu);
+ else
+ printf("Error exchanging MTU\n");
+}
+
+static void cmd_mtu(int argcp, char **argvp)
+{
+ if (conn_state != STATE_CONNECTED) {
+ printf("Command failed: not connected.\n");
+ return;
+ }
+
+ if (opt_psm) {
+ printf("Command failed: operation is only available for LE"
+ " transport.\n");
+ return;
+ }
+
+ if (argcp < 2) {
+ printf("Usage: mtu <value>\n");
+ return;
+ }
+
+ if (opt_mtu) {
+ printf("Command failed: MTU exchange can only occur once per"
+ " connection.\n");
+ return;
+ }
+
+ errno = 0;
+ opt_mtu = strtoll(argvp[1], NULL, 0);
+ if (errno != 0 || opt_mtu < ATT_DEFAULT_LE_MTU) {
+ printf("Invalid value. Minimum MTU size is %d\n",
+ ATT_DEFAULT_LE_MTU);
+ return;
+ }
+
+ gatt_exchange_mtu(attrib, opt_mtu, exchange_mtu_cb, NULL);
+}
+
static struct {
const char *cmd;
void (*func)(int argcp, char **argvp);
@@ -684,6 +744,8 @@ static struct {
"Characteristic Value Write (No response)" },
{ "sec-level", cmd_sec_level, "[low | medium | high]",
"Set security level. Default: low" },
+ { "mtu", cmd_mtu, "<value>",
+ "Exchange MTU for GATT/ATT" },
{ NULL, NULL, NULL}
};

--
1.7.0.4


2011-03-17 14:55:19

by Bruna Moreira

[permalink] [raw]
Subject: [PATCHv2 4/5] Add Exchange MTU operation in GATT library

---
attrib/gatt.c | 13 +++++++++++++
attrib/gatt.h | 3 +++
2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index 28654af..0b69daf 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -534,6 +534,19 @@ guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
user_data, NULL);
}

+guint gatt_exchange_mtu(GAttrib *attrib, uint16_t mtu, GAttribResultFunc func,
+ gpointer user_data)
+{
+ uint8_t *buf;
+ int buflen;
+ guint16 plen;
+
+ buf = g_attrib_get_buffer(attrib, &buflen);
+ plen = enc_mtu_req(mtu, buf, buflen);
+ return g_attrib_send(attrib, 0, ATT_OP_MTU_REQ, buf, plen, func,
+ user_data, NULL);
+}
+
guint gatt_find_info(GAttrib *attrib, uint16_t start, uint16_t end,
GAttribResultFunc func, gpointer user_data)
{
diff --git a/attrib/gatt.h b/attrib/gatt.h
index c6d3843..221d94d 100644
--- a/attrib/gatt.h
+++ b/attrib/gatt.h
@@ -48,3 +48,6 @@ guint gatt_write_cmd(GAttrib *attrib, uint16_t handle, uint8_t *value, int vlen,
guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
bt_uuid_t *uuid, GAttribResultFunc func,
gpointer user_data);
+
+guint gatt_exchange_mtu(GAttrib *attrib, uint16_t mtu, GAttribResultFunc func,
+ gpointer user_data);
--
1.7.0.4


2011-03-17 14:55:18

by Bruna Moreira

[permalink] [raw]
Subject: [PATCHv2 3/5] Use GAttrib buffer for ATT protocol PDUs

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 1cd651c..28654af 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -109,9 +109,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;
@@ -130,13 +130,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;

@@ -197,12 +198,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;
}
@@ -216,11 +218,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;

@@ -238,7 +241,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,
@@ -247,7 +250,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;
@@ -297,15 +301,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;
@@ -320,14 +326,15 @@ guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
bt_uuid_t *uuid, 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;
bt_uuid_t type_uuid;
guint16 plen;

bt_uuid16_create(&type_uuid, GATT_CHARAC_UUID);

- plen = enc_read_by_type_req(start, end, &type_uuid, pdu, sizeof(pdu));
+ plen = enc_read_by_type_req(start, end, &type_uuid, buf, buflen);
if (plen == 0)
return 0;

@@ -341,7 +348,7 @@ guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
dc->end = end;
dc->uuid = g_memdup(uuid, sizeof(bt_uuid_t));

- 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);
}

@@ -349,15 +356,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 {
@@ -388,7 +396,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;
@@ -409,13 +418,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) {
@@ -434,11 +444,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);
@@ -449,9 +460,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) {
@@ -468,7 +479,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;
@@ -483,14 +495,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);
}

@@ -507,39 +520,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


2011-03-17 14:55:17

by Bruna Moreira

[permalink] [raw]
Subject: [PATCHv2 2/5] Add internal buffer to GAttrib struct

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..290cd96 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_mtu(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..4c49879 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_mtu(GAttrib *attrib, int mtu);
+
gboolean g_attrib_unregister(GAttrib *attrib, guint id);
gboolean g_attrib_unregister_all(GAttrib *attrib);

--
1.7.0.4


2011-03-17 14:55:16

by Bruna Moreira

[permalink] [raw]
Subject: [PATCHv2 1/5] TODO: remove 'fix MTU exchange' task

---
TODO | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/TODO b/TODO
index f49a9c4..588d9f6 100644
--- a/TODO
+++ b/TODO
@@ -130,11 +130,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


2011-03-17 12:43:14

by Bruna Moreira

[permalink] [raw]
Subject: Re: [PATCH 2/5] Add internal buffer to GAttrib struct

On 3/17/11, Johan Hedberg <[email protected]> wrote:
> Hi Bruna,
>
> On Wed, Mar 16, 2011, Bruna Moreira wrote:
>> +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;
>> +}
>
> Wouldn't g_attrib_set_mtu make more sense for the name of this function?
> Since it's named symmetrically to the _get version one would expect it
> to take a buffer as an input parameter, but it's not doing that.

Agreed. I will send a new updated series.

BR,
--
Bruna Moreira
Instituto Nokia de Tecnologia (INdT)
Manaus - Brazil

2011-03-17 12:30:31

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 2/5] Add internal buffer to GAttrib struct

Hi Bruna,

On Wed, Mar 16, 2011, Bruna Moreira wrote:
> +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;
> +}

Wouldn't g_attrib_set_mtu make more sense for the name of this function?
Since it's named symmetrically to the _get version one would expect it
to take a buffer as an input parameter, but it's not doing that.

Johan

2011-03-16 11:25:12

by Bruna Moreira

[permalink] [raw]
Subject: [PATCH 5/5] Add Exchange MTU in interactive gatttool

---
attrib/interactive.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/attrib/interactive.c b/attrib/interactive.c
index b32e9e7..38fc75a 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -362,6 +362,7 @@ static void cmd_disconnect(int argcp, char **argvp)

g_attrib_unref(attrib);
attrib = NULL;
+ opt_mtu = 0;

g_io_channel_shutdown(iochannel, FALSE, NULL);
g_io_channel_unref(iochannel);
@@ -642,6 +643,65 @@ static void cmd_sec_level(int argcp, char **argvp)
}
}

+static void exchange_mtu_cb(guint8 status, const guint8 *pdu, guint16 plen,
+ gpointer user_data)
+{
+ uint16_t mtu;
+
+ if (status != 0) {
+ printf("Exchange MTU Request failed: %s\n",
+ att_ecode2str(status));
+ return;
+ }
+
+ if (!dec_mtu_resp(pdu, plen, &mtu)) {
+ printf("Protocol error\n");
+ return;
+ }
+
+ mtu = MIN(mtu, opt_mtu);
+ /* Set new value for MTU in client */
+ if (g_attrib_set_buffer(attrib, mtu))
+ printf("MTU was exchanged successfully: %d\n", mtu);
+ else
+ printf("Error exchanging MTU\n");
+}
+
+static void cmd_mtu(int argcp, char **argvp)
+{
+ if (conn_state != STATE_CONNECTED) {
+ printf("Command failed: not connected.\n");
+ return;
+ }
+
+ if (opt_psm) {
+ printf("Command failed: operation is only available for LE"
+ " transport.\n");
+ return;
+ }
+
+ if (argcp < 2) {
+ printf("Usage: mtu <value>\n");
+ return;
+ }
+
+ if (opt_mtu) {
+ printf("Command failed: MTU exchange can only occur once per"
+ " connection.\n");
+ return;
+ }
+
+ errno = 0;
+ opt_mtu = strtoll(argvp[1], NULL, 0);
+ if (errno != 0 || opt_mtu < ATT_DEFAULT_LE_MTU) {
+ printf("Invalid value. Minimum MTU size is %d\n",
+ ATT_DEFAULT_LE_MTU);
+ return;
+ }
+
+ gatt_exchange_mtu(attrib, opt_mtu, exchange_mtu_cb, NULL);
+}
+
static struct {
const char *cmd;
void (*func)(int argcp, char **argvp);
@@ -672,6 +732,8 @@ static struct {
"Characteristic Value Write (No response)" },
{ "sec-level", cmd_sec_level, "[low | medium | high]",
"Set security level. Default: low" },
+ { "mtu", cmd_mtu, "<value>",
+ "Exchange MTU for GATT/ATT" },
{ NULL, NULL, NULL}
};

--
1.7.0.4


2011-03-16 11:25:11

by Bruna Moreira

[permalink] [raw]
Subject: [PATCH 4/5] Add Exchange MTU operation in GATT library

---
attrib/gatt.c | 13 +++++++++++++
attrib/gatt.h | 3 +++
2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index d3333fd..a9863e3 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -528,6 +528,19 @@ guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
user_data, NULL);
}

+guint gatt_exchange_mtu(GAttrib *attrib, uint16_t mtu, GAttribResultFunc func,
+ gpointer user_data)
+{
+ uint8_t *buf;
+ int buflen;
+ guint16 plen;
+
+ buf = g_attrib_get_buffer(attrib, &buflen);
+ plen = enc_mtu_req(mtu, buf, buflen);
+ return g_attrib_send(attrib, 0, ATT_OP_MTU_REQ, buf, plen, func,
+ user_data, NULL);
+}
+
guint gatt_find_info(GAttrib *attrib, uint16_t start, uint16_t end,
GAttribResultFunc func, gpointer user_data)
{
diff --git a/attrib/gatt.h b/attrib/gatt.h
index 730de7e..347c1a6 100644
--- a/attrib/gatt.h
+++ b/attrib/gatt.h
@@ -47,3 +47,6 @@ guint gatt_write_cmd(GAttrib *attrib, uint16_t handle, uint8_t *value, int vlen,
guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
bt_uuid_t *uuid, GAttribResultFunc func,
gpointer user_data);
+
+guint gatt_exchange_mtu(GAttrib *attrib, uint16_t mtu, GAttribResultFunc func,
+ gpointer user_data);
--
1.7.0.4


2011-03-16 11:25:10

by Bruna Moreira

[permalink] [raw]
Subject: [PATCH 3/5] Use GAttrib buffer for ATT protocol PDUs

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


2011-03-16 11:25:09

by Bruna Moreira

[permalink] [raw]
Subject: [PATCH 2/5] Add internal buffer to GAttrib struct

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