2014-05-21 14:17:31

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv4 0/4] MTU negotiation

V4 changes:
* add check for application type allowing only client to send request
* simplified mtu handling, its stored in device on every change so there
is no need to take it out of attrib io channel every time
* added patches 3 and 4

Jakub Tyszkowski (4):
android/gatt: Send Exchange MTU request on connect
android/gatt: Use g_attrib buffer where possible for att
android/gatt: Make att_handler use g_attrib buffer
android/gatt: Use default mtu on mtu exchange response

android/gatt.c | 221 +++++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 168 insertions(+), 53 deletions(-)

--
1.9.3



2014-05-21 14:17:35

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv4 4/4] android/gatt: Use default mtu on mtu exchange response

We should be setting new mtu after sending response using old mtu. This
also moves sending the response from att_handler to mtu_att_handle.
---
android/gatt.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 8261807..e3cbc88 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4540,14 +4540,14 @@ static uint8_t read_request(const uint8_t *cmd, uint16_t cmd_len,
}

static uint8_t mtu_att_handle(const uint8_t *cmd, uint16_t cmd_len,
- uint8_t *rsp, size_t rsp_size,
- struct gatt_device *dev,
- uint16_t *length)
+ struct gatt_device *dev)
{
- uint16_t mtu, imtu, omtu;
GIOChannel *io;
GError *gerr = NULL;
+ uint16_t mtu, imtu;
+ size_t omtu;
uint16_t len;
+ uint8_t *rsp;

DBG("");

@@ -4562,7 +4562,6 @@ static uint8_t mtu_att_handle(const uint8_t *cmd, uint16_t cmd_len,

bt_io_get(io, &gerr,
BT_IO_OPT_IMTU, &imtu,
- BT_IO_OPT_OMTU, &omtu,
BT_IO_OPT_INVALID);
if (gerr) {
error("bt_io_get: %s", gerr->message);
@@ -4570,16 +4569,18 @@ static uint8_t mtu_att_handle(const uint8_t *cmd, uint16_t cmd_len,
return ATT_ECODE_UNLIKELY;
}

- /* Limit OMTU to received value */
- dev->mtu = MIN(mtu, omtu);
- g_attrib_set_mtu(dev->attrib, dev->mtu);
+ rsp = g_attrib_get_buffer(dev->attrib, &omtu);

/* Respond with our IMTU */
- len = enc_mtu_resp(imtu, rsp, rsp_size);
+ len = enc_mtu_resp(imtu, rsp, omtu);
if (!len)
return ATT_ECODE_UNLIKELY;

- *length = len;
+ g_attrib_send(dev->attrib, 0, rsp, len, NULL, NULL, NULL);
+
+ /* Limit OMTU to received value */
+ dev->mtu = MIN(mtu, omtu);
+ g_attrib_set_mtu(dev->attrib, dev->mtu);

return 0;
}
@@ -4847,8 +4848,7 @@ static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data)
status = read_request(ipdu, len, dev);
break;
case ATT_OP_MTU_REQ:
- status = mtu_att_handle(ipdu, len, opdu, length, dev,
- &resp_length);
+ status = mtu_att_handle(ipdu, len, dev);
break;
case ATT_OP_FIND_INFO_REQ:
status = find_info_handle(ipdu, len, opdu, length,
--
1.9.3


2014-05-21 14:17:34

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv4 3/4] android/gatt: Make att_handler use g_attrib buffer

This makes use of g_attrib buffer for output and removes some unused
read_by_group_type function parameters.
---
android/gatt.c | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index c231c95..8261807 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4384,8 +4384,6 @@ static const struct ipc_handler cmd_handlers[] = {
};

static uint8_t read_by_group_type(const uint8_t *cmd, uint16_t cmd_len,
- uint8_t *rsp, size_t rsp_size,
- uint16_t *length,
struct gatt_device *device)
{
uint16_t start, end;
@@ -4824,24 +4822,22 @@ static uint8_t write_prep_request(const uint8_t *cmd, uint16_t cmd_len,
static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data)
{
struct gatt_device *dev = user_data;
- uint8_t opdu[ATT_DEFAULT_LE_MTU];
uint8_t status;
- uint16_t length = 0;
- size_t vlen;
- uint8_t *value = g_attrib_get_buffer(dev->attrib, &vlen);
+ uint16_t resp_length = 0;
+ size_t length;
+ uint8_t *opdu = g_attrib_get_buffer(dev->attrib, &length);

DBG("op 0x%02x", ipdu[0]);

- if (len > vlen) {
- error("gatt: Too much data on ATT socket %p", value);
+ if (len > length) {
+ error("gatt: Too much data on ATT socket %p", opdu);
status = ATT_ECODE_INVALID_PDU;
goto done;
}

switch (ipdu[0]) {
case ATT_OP_READ_BY_GROUP_REQ:
- status = read_by_group_type(ipdu, len, opdu, sizeof(opdu),
- &length, dev);
+ status = read_by_group_type(ipdu, len, dev);
break;
case ATT_OP_READ_BY_TYPE_REQ:
status = read_by_type(ipdu, len, dev);
@@ -4851,12 +4847,12 @@ static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data)
status = read_request(ipdu, len, dev);
break;
case ATT_OP_MTU_REQ:
- status = mtu_att_handle(ipdu, len, opdu, sizeof(opdu), dev,
- &length);
+ status = mtu_att_handle(ipdu, len, opdu, length, dev,
+ &resp_length);
break;
case ATT_OP_FIND_INFO_REQ:
- status = find_info_handle(ipdu, len, opdu, sizeof(opdu),
- &length);
+ status = find_info_handle(ipdu, len, opdu, length,
+ &resp_length);
break;
case ATT_OP_WRITE_REQ:
status = write_req_request(ipdu, len, dev);
@@ -4891,11 +4887,12 @@ static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data)

done:
if (status)
- length = enc_error_resp(ipdu[0], 0x0000, status, opdu,
- ATT_DEFAULT_LE_MTU);
+ resp_length = enc_error_resp(ipdu[0], 0x0000, status, opdu,
+ length);

- if (length)
- g_attrib_send(dev->attrib, 0, opdu, length, NULL, NULL, NULL);
+ if (resp_length)
+ g_attrib_send(dev->attrib, 0, opdu, resp_length, NULL, NULL,
+ NULL);
}

static void create_listen_connections(void *data, void *user_data)
--
1.9.3


2014-05-21 14:17:33

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv4 2/4] android/gatt: Use g_attrib buffer where possible for att

This replaces fixed size pdu usage with g_attrib buffer when possible.
When only received packets are decoded we use dynamic allocation with
current mtu.
---
android/gatt.c | 111 ++++++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 86 insertions(+), 25 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index e9db9e6..c231c95 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -3553,7 +3553,8 @@ static bool is_service(const bt_uuid_t *type)
static void send_dev_pending_response(struct gatt_device *device,
uint8_t opcode)
{
- uint8_t rsp[ATT_DEFAULT_LE_MTU];
+ size_t mtu;
+ uint8_t *rsp = g_attrib_get_buffer(device->attrib, &mtu);
struct pending_request *val;
uint16_t len = 0;
uint8_t error = 0;
@@ -3598,7 +3599,7 @@ static void send_dev_pending_response(struct gatt_device *device,
val = queue_pop_head(temp);
}

- len = enc_read_by_type_resp(adl, rsp, sizeof(rsp));
+ len = enc_read_by_type_resp(adl, rsp, mtu);

att_data_list_free(adl);
queue_destroy(temp, destroy_pending_request);
@@ -3613,7 +3614,7 @@ static void send_dev_pending_response(struct gatt_device *device,
}

len = enc_read_blob_resp(val->value, val->length, val->offset,
- rsp, sizeof(rsp));
+ rsp, mtu);
destroy_pending_request(val);
break;
case ATT_OP_READ_REQ:
@@ -3623,7 +3624,7 @@ static void send_dev_pending_response(struct gatt_device *device,
goto done;
}

- len = enc_read_resp(val->value, val->length, rsp, sizeof(rsp));
+ len = enc_read_resp(val->value, val->length, rsp, mtu);
destroy_pending_request(val);
break;
case ATT_OP_READ_BY_GROUP_REQ: {
@@ -3669,7 +3670,7 @@ static void send_dev_pending_response(struct gatt_device *device,
val = queue_pop_head(temp);
}

- len = enc_read_by_grp_resp(adl, rsp, sizeof(rsp));
+ len = enc_read_by_grp_resp(adl, rsp, mtu);

att_data_list_free(adl);
queue_destroy(temp, destroy_pending_request);
@@ -3717,7 +3718,7 @@ static void send_dev_pending_response(struct gatt_device *device,
}

if (list && !error)
- len = enc_find_by_type_resp(list, rsp, sizeof(rsp));
+ len = enc_find_by_type_resp(list, rsp, mtu);
else
error = ATT_ECODE_ATTR_NOT_FOUND;

@@ -3753,7 +3754,7 @@ static void send_dev_pending_response(struct gatt_device *device,
}

len = enc_prep_write_resp(val->handle, val->offset, val->value,
- val->length, rsp, sizeof(rsp));
+ val->length, rsp, mtu);
destroy_pending_request(val);
break;
default:
@@ -3762,8 +3763,7 @@ static void send_dev_pending_response(struct gatt_device *device,

done:
if (!len)
- len = enc_error_resp(opcode, 0x0000, error, rsp,
- ATT_DEFAULT_LE_MTU);
+ len = enc_error_resp(opcode, 0x0000, error, rsp, mtu);

g_attrib_send(device->attrib, 0, rsp, len, NULL, NULL, NULL);

@@ -4199,10 +4199,11 @@ failed:
static void handle_server_send_indication(const void *buf, uint16_t len)
{
const struct hal_cmd_gatt_server_send_indication *cmd = buf;
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
struct app_connection *conn;
uint8_t status;
uint16_t length;
+ uint8_t *pdu;
+ size_t mtu;

DBG("");

@@ -4213,15 +4214,17 @@ static void handle_server_send_indication(const void *buf, uint16_t len)
goto reply;
}

+ pdu = g_attrib_get_buffer(conn->device->attrib, &mtu);
+
if (cmd->confirm)
/* TODO: Add data to track confirmation for this request */
length = enc_indication(cmd->attribute_handle,
- (uint8_t *)cmd->value, cmd->len,
- pdu, sizeof(pdu));
+ (uint8_t *)cmd->value, cmd->len, pdu,
+ mtu);
else
length = enc_notification(cmd->attribute_handle,
(uint8_t *)cmd->value, cmd->len,
- pdu, sizeof(pdu));
+ pdu, mtu);

g_attrib_send(conn->device->attrib, 0, pdu, length, NULL, NULL, NULL);

@@ -4653,7 +4656,7 @@ static uint8_t find_info_handle(const uint8_t *cmd, uint16_t cmd_len,
static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
struct gatt_device *device)
{
- uint8_t search_value[ATT_DEFAULT_LE_MTU];
+ uint8_t *search_value;
size_t search_vlen;
uint16_t start, end;
uint16_t handle;
@@ -4663,14 +4666,24 @@ static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,

DBG("");

+ search_value = malloc0(device->mtu);
+ if (!search_value)
+ return ATT_ECODE_INSUFF_RESOURCES;
+
len = dec_find_by_type_req(cmd, cmd_len, &start, &end, &uuid,
search_value, &search_vlen);
- if (!len)
+ if (!len) {
+ free(search_value);
+
return ATT_ECODE_INVALID_PDU;
+ }

q = queue_new();
- if (!q)
+ if (!q) {
+ free(search_value);
+
return ATT_ECODE_UNLIKELY;
+ }

gatt_db_find_by_type(gatt_db, start, end, &uuid, q);

@@ -4681,6 +4694,8 @@ static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
data = new0(struct pending_request, 1);
if (!data) {
queue_destroy(q, NULL);
+ free(search_value);
+
return ATT_ECODE_INSUFF_RESOURCES;
}

@@ -4688,6 +4703,8 @@ static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
if (!data) {
destroy_pending_request(data);
queue_destroy(q, NULL);
+ free(search_value);
+
return ATT_ECODE_INSUFF_RESOURCES;
}

@@ -4702,6 +4719,7 @@ static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
}

queue_destroy(q, NULL);
+ free(search_value);

process_dev_pending_requests(device, ATT_OP_FIND_BY_TYPE_REQ);

@@ -4711,14 +4729,30 @@ static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
static void write_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
struct gatt_device *dev)
{
- uint8_t value[ATT_DEFAULT_LE_MTU];
+ uint8_t *value;
uint16_t handle;
uint16_t len;
size_t vlen;

+ value = malloc0(dev->mtu);
+ if (!value)
+ return;
+
len = dec_write_cmd(cmd, cmd_len, &handle, value, &vlen);
- if (!len)
+ if (!len) {
+ free(value);
+
+ return;
+ }
+
+ if (!gatt_db_write(gatt_db, handle, 0, value, vlen, cmd[0],
+ &dev->bdaddr)) {
+ free(value);
+
return;
+ }
+
+ free(value);

gatt_db_write(gatt_db, handle, 0, value, vlen, cmd[0], &dev->bdaddr);
}
@@ -4726,18 +4760,30 @@ static void write_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
static uint8_t write_req_request(const uint8_t *cmd, uint16_t cmd_len,
struct gatt_device *dev)
{
- uint8_t value[ATT_DEFAULT_LE_MTU];
+ uint8_t *value;
uint16_t handle;
uint16_t len;
size_t vlen;

+ value = malloc0(dev->mtu);
+ if (!value)
+ return ATT_ECODE_INSUFF_RESOURCES;
+
len = dec_write_req(cmd, cmd_len, &handle, value, &vlen);
- if (!len)
+ if (!len) {
+ free(value);
+
return ATT_ECODE_INVALID_PDU;
+ }

if (!gatt_db_write(gatt_db, handle, 0, value, vlen, cmd[0],
- &dev->bdaddr))
+ &dev->bdaddr)) {
+ free(value);
+
return ATT_ECODE_UNLIKELY;
+ }
+
+ free(value);

return 0;
}
@@ -4745,20 +4791,32 @@ static uint8_t write_req_request(const uint8_t *cmd, uint16_t cmd_len,
static uint8_t write_prep_request(const uint8_t *cmd, uint16_t cmd_len,
struct gatt_device *dev)
{
- uint8_t value[ATT_DEFAULT_LE_MTU];
+ uint8_t *value;
uint16_t handle;
uint16_t offset;
uint16_t len;
size_t vlen;

+ value = malloc0(dev->mtu);
+ if (!value)
+ return ATT_ECODE_INSUFF_RESOURCES;
+
len = dec_prep_write_req(cmd, cmd_len, &handle, &offset,
value, &vlen);
- if (!len)
+ if (!len) {
+ free(value);
+
return ATT_ECODE_INVALID_PDU;
+ }

if (!gatt_db_write(gatt_db, handle, offset, value, vlen, cmd[0],
- &dev->bdaddr))
+ &dev->bdaddr)) {
+ free(value);
+
return ATT_ECODE_UNLIKELY;
+ }
+
+ free(value);

return 0;
}
@@ -5127,9 +5185,10 @@ static void gatt_srvc_change_register_cb(uint16_t handle, uint16_t offset,
bdaddr_t *bdaddr,
void *user_data)
{
- uint8_t pdu[ATT_DEFAULT_LE_MTU];
struct gatt_device *dev;
uint16_t length;
+ size_t mtu;
+ uint8_t *pdu;

dev = find_device_by_addr(bdaddr);
if (!dev) {
@@ -5137,6 +5196,8 @@ static void gatt_srvc_change_register_cb(uint16_t handle, uint16_t offset,
return;
}

+ pdu = g_attrib_get_buffer(dev->attrib, &mtu);
+
/* TODO handle CCC */

/* Set services changed notification flag */
--
1.9.3


2014-05-21 14:17:32

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCHv4 1/4] android/gatt: Send Exchange MTU request on connect

This adds sending exchange MTU request on connection.

This is needed to pass TC_GAC_CL_BV_01_C.
---
android/gatt.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 59 insertions(+), 2 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 89da60d..e9db9e6 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -144,6 +144,7 @@ struct gatt_device {

gatt_device_state_t state;

+ size_t mtu;
GAttrib *attrib;
GIOChannel *att_io;
struct queue *services;
@@ -368,6 +369,15 @@ static struct app_connection *find_connection_by_id(int32_t conn_id)
INT_TO_PTR(conn_id));
}

+static bool match_client_connection_by_device(const void *data,
+ const void *user_data)
+{
+ const struct app_connection *conn = data;
+ const struct gatt_device *dev = user_data;
+
+ return conn->device == dev ? conn->app->type == APP_CLIENT : false;
+}
+
static bool match_connection_by_device(const void *data, const void *user_data)
{
const struct app_connection *conn = data;
@@ -979,6 +989,48 @@ static void send_app_connect_notifications(void *data, void *user_data)

static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data);

+static void exchange_mtu_cb(guint8 status, const guint8 *pdu, guint16 plen,
+ gpointer user_data)
+{
+ struct gatt_device *device = user_data;
+ uint16_t rmtu, mtu;
+
+ if (status) {
+ error("gatt: MTU exchange: %s", att_ecode2str(status));
+ goto failed;
+ }
+
+ if (!dec_mtu_resp(pdu, plen, &rmtu)) {
+ error("gatt: MTU exchange: protocol error");
+ goto failed;
+ }
+
+ if (rmtu < ATT_DEFAULT_LE_MTU) {
+ error("gatt: MTU exchange: mtu error");
+ goto failed;
+ }
+
+ mtu = MIN(rmtu, device->mtu);
+ if (mtu != device->mtu && !g_attrib_set_mtu(device->attrib, mtu)) {
+ error("gatt: MTU exchange failed");
+ goto failed;
+ }
+
+ DBG("MTU exchange succeeded: rmtu:%d, old mtu:%zd, new mtu:%d", rmtu,
+ device->mtu, mtu);
+ device->mtu = mtu;
+
+failed:
+ device_unref(device);
+}
+
+static void send_exchange_mtu_request(struct gatt_device *device)
+{
+ if (!gatt_exchange_mtu(device->attrib, device->mtu,
+ exchange_mtu_cb, device_ref(device)))
+ device_unref(device);
+}
+
static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
{
struct gatt_device *dev = user_data;
@@ -1022,10 +1074,15 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
if (dev->server_id == 0)
error("gatt: Could not attach to server");

+ /* store current mtu */
+ g_attrib_get_buffer(attrib, &dev->mtu);
device_set_state(dev, DEVICE_CONNECTED);

status = GATT_SUCCESS;

+ if (queue_find(app_connections, match_client_connection_by_device, dev))
+ send_exchange_mtu_request(dev);
+
reply:
data.dev = dev;
data.status = status;
@@ -4513,8 +4570,8 @@ static uint8_t mtu_att_handle(const uint8_t *cmd, uint16_t cmd_len,
}

/* Limit OMTU to received value */
- mtu = MIN(mtu, omtu);
- g_attrib_set_mtu(dev->attrib, mtu);
+ dev->mtu = MIN(mtu, omtu);
+ g_attrib_set_mtu(dev->attrib, dev->mtu);

/* Respond with our IMTU */
len = enc_mtu_resp(imtu, rsp, rsp_size);
--
1.9.3