Hi Arman,
On Fri, Nov 7, 2014 at 10:41 PM, Arman Uguray <armansito@xxxxxxxxxxxx> wrote:
> This patch implements the ATT protocol "Read By Type" request for
> shared/gatt-server. Logic is implemented that allows asynchronous
> reading of non-standard attribute values via the registered read and
> read completion callbacks.
> ---
> src/shared/gatt-server.c | 246 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 243 insertions(+), 3 deletions(-)
>
> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
> index 18f82c4..ddb0be1 100644
> --- a/src/shared/gatt-server.c
> +++ b/src/shared/gatt-server.c
> @@ -40,6 +40,16 @@
> #define MIN(a, b) ((a) < (b) ? (a) : (b))
> #endif
>
> +struct async_read_op {
> + struct bt_gatt_server *server;
> + uint8_t opcode;
> + bool done;
> + uint8_t *pdu;
> + size_t pdu_len;
> + size_t value_len;
> + struct queue *db_data;
> +};
> +
> struct bt_gatt_server {
> struct gatt_db *db;
> struct bt_att *att;
> @@ -48,6 +58,9 @@ struct bt_gatt_server {
>
> unsigned int mtu_id;
> unsigned int read_by_grp_type_id;
> + unsigned int read_by_type_id;
> +
> + struct async_read_op *pending_read_op;
>
> bt_gatt_server_debug_func_t debug_callback;
> bt_gatt_server_destroy_func_t debug_destroy;
> @@ -61,11 +74,23 @@ static void bt_gatt_server_free(struct bt_gatt_server *server)
>
> bt_att_unregister(server->att, server->mtu_id);
> bt_att_unregister(server->att, server->read_by_grp_type_id);
> + bt_att_unregister(server->att, server->read_by_type_id);
> bt_att_unref(server->att);
>
> + if (server->pending_read_op)
> + server->pending_read_op->server = NULL;
> +
> free(server);
> }
>
> +static uint8_t att_ecode_from_error(int err)
> +{
> + if (err < 0 || err > UINT8_MAX)
> + return 0xff;
> +
> + return err;
> +}
> +
> static void encode_error_rsp(uint8_t opcode, uint16_t handle, uint8_t ecode,
> uint8_t pdu[4])
> {
> @@ -136,14 +161,15 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
> * value is seen.
> */
> if (iter == 0) {
> - data_val_len = value.iov_len;
> + data_val_len = MIN(MIN((unsigned)mtu - 6, 251),
> + value.iov_len);
> pdu[0] = data_val_len + 4;
> iter++;
> } else if (value.iov_len != data_val_len)
> break;
>
> /* Stop if this unit would surpass the MTU */
> - if (iter + data_val_len + 4 > mtu)
> + if (iter + data_val_len + 4 > mtu - 1)
> break;
>
> gatt_db_attribute_get_service_handles(attrib, &start_handle,
> @@ -151,7 +177,7 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
>
> put_le16(start_handle, pdu + iter);
> put_le16(end_handle, pdu + iter + 2);
> - memcpy(pdu + iter + 4, value.iov_base, value.iov_len);
> + memcpy(pdu + iter + 4, value.iov_base, data_val_len);
>
> iter += data_val_len + 4;
> }
This seems to be unrelated to the patch, it looks like a fix or
perhaps multiple fixes.
> @@ -248,6 +274,212 @@ done:
> NULL, NULL, NULL);
> }
>
> +static void async_read_op_destroy(struct async_read_op *op)
> +{
> + if (op->server)
> + op->server->pending_read_op = NULL;
> +
> + queue_destroy(op->db_data, NULL);
> + free(op->pdu);
> + free(op);
> +}
> +
> +static void process_read_by_type(struct async_read_op *op);
> +
> +static void read_by_type_read_complete_cb(struct gatt_db_attribute *attr,
> + int err, const uint8_t *value,
> + size_t len, void *user_data)
> +{
> + struct async_read_op *op = user_data;
> + struct bt_gatt_server *server = op->server;
> + uint16_t mtu;
> + uint16_t handle;
> +
> + if (!server) {
> + async_read_op_destroy(op);
> + return;
> + }
> +
> + mtu = bt_att_get_mtu(server->att);
> + handle = gatt_db_attribute_get_handle(attr);
> +
> + /* Terminate the operation if there was an error */
> + if (err) {
> + uint8_t pdu[4];
> + uint8_t att_ecode = att_ecode_from_error(err);
> +
> + encode_error_rsp(BT_ATT_OP_READ_BY_TYPE_REQ, handle, att_ecode,
> + pdu);
> + bt_att_send(server->att, BT_ATT_OP_ERROR_RSP, pdu, 4, NULL,
> + NULL, NULL);
> + async_read_op_destroy(op);
> + return;
> + }
> +
> + if (op->pdu_len == 0) {
> + op->value_len = MIN(MIN((unsigned) mtu - 4, 253), len);
> + op->pdu[0] = op->value_len + 2;
> + op->pdu_len++;
> + } else if (len != op->value_len) {
> + op->done = true;
> + goto done;
> + }
> +
> + /* Stop if this would surpass the MTU */
> + if (op->pdu_len + op->value_len + 2 > (unsigned) mtu - 1) {
> + op->done = true;
> + goto done;
> + }
> +
> + /* Encode the current value */
> + put_le16(handle, op->pdu + op->pdu_len);
> + memcpy(op->pdu + op->pdu_len + 2, value, op->value_len);
> +
> + op->pdu_len += op->value_len + 2;
> +
> + if (op->pdu_len == (unsigned) mtu - 1)
> + op->done = true;
> +
> +done:
> + process_read_by_type(op);
> +}
> +
> +static void process_read_by_type(struct async_read_op *op)
> +{
> + struct bt_gatt_server *server = op->server;
> + uint8_t rsp_opcode;
> + uint8_t rsp_len;
> + uint8_t ecode;
> + uint16_t ehandle;
> + struct gatt_db_attribute *attr;
> + uint32_t perm;
> +
> + attr = queue_pop_head(op->db_data);
> +
> + if (op->done || !attr) {
> + rsp_opcode = BT_ATT_OP_READ_BY_TYPE_RSP;
> + rsp_len = op->pdu_len;
> + goto done;
> + }
> +
> + if (!gatt_db_attribute_get_permissions(attr, &perm)) {
> + ecode = BT_ATT_ERROR_UNLIKELY;
> + goto error;
> + }
> +
> + /*
> + * Check for the READ access permission. Encryption,
> + * authentication, and authorization permissions need to be
> + * checked by the read handler, since bt_att is agnostic to
> + * connection type and doesn't have security information on it.
> + */
> + if (perm && !(perm & BT_ATT_PERM_READ)) {
> + ecode = BT_ATT_ERROR_READ_NOT_PERMITTED;
> + goto error;
> + }
> +
> + if (gatt_db_attribute_read(attr, 0, op->opcode, NULL,
> + read_by_type_read_complete_cb, op))
> + return;
> +
> + ecode = BT_ATT_ERROR_UNLIKELY;
> +
> +error:
> + ehandle = gatt_db_attribute_get_handle(attr);
> + rsp_opcode = BT_ATT_OP_ERROR_RSP;
> + rsp_len = 4;
> + encode_error_rsp(BT_ATT_OP_READ_BY_TYPE_REQ, ehandle, ecode, op->pdu);
> +
> +done:
> + bt_att_send(server->att, rsp_opcode, op->pdu, rsp_len, NULL,
> + NULL, NULL);
> + async_read_op_destroy(op);
> +}
> +
> +static void read_by_type_cb(uint8_t opcode, const void *pdu,
> + uint16_t length, void *user_data)
> +{
> + struct bt_gatt_server *server = user_data;
> + uint16_t start, end;
> + bt_uuid_t type;
> + uint8_t rsp_pdu[4];
> + uint16_t ehandle = 0;
> + uint8_t ecode;
> + struct queue *q = NULL;
> + struct async_read_op *op;
> +
> + if (length != 6 && length != 20) {
> + ecode = BT_ATT_ERROR_INVALID_PDU;
> + goto error;
> + }
> +
> + q = queue_new();
> + if (!q) {
> + ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> + goto error;
> + }
> +
> + start = get_le16(pdu);
> + end = get_le16(pdu + 2);
> + get_uuid_le(pdu + 4, length - 4, &type);
> +
> + util_debug(server->debug_callback, server->debug_data,
> + "Read By Type - start: 0x%04x end: 0x%04x",
> + start, end);
> +
> + if (!start || !end) {
> + ecode = BT_ATT_ERROR_INVALID_HANDLE;
> + goto error;
> + }
> +
> + ehandle = start;
> +
> + if (start > end) {
> + ecode = BT_ATT_ERROR_INVALID_HANDLE;
> + goto error;
> + }
> +
> + gatt_db_read_by_type(server->db, start, end, type, q);
> +
> + if (queue_isempty(q)) {
> + ecode = BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND;
> + goto error;
> + }
> +
> + if (server->pending_read_op) {
> + ecode = BT_ATT_ERROR_UNLIKELY;
> + goto error;
> + }
> +
> + op = new0(struct async_read_op, 1);
> + if (!op) {
> + ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> + goto error;
> + }
> +
> + op->pdu = malloc(bt_att_get_mtu(server->att));
> + if (!op->pdu) {
> + free(op);
> + ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> + goto error;
> + }
Lately Ive been trying to advocate for using iovec whenever possible,
for example here we are doing 2 copies for each attribute when we
could be just passing references and only copy if the PDU has to be
queued, perhaps because the ATT PDUs are rather small we did not think
this would be a problem but for more complex operations such as this I
think we should perhaps reconsider about this strategy. Anyway, we can
add to the TODO since right now this is not critical.
> + op->opcode = opcode;
> + op->server = server;
> + op->db_data = q;
> + server->pending_read_op = op;
> +
> + process_read_by_type(op);
> +
> + return;
> +
> +error:
> + encode_error_rsp(opcode, ehandle, ecode, rsp_pdu);
> + queue_destroy(q, NULL);
> + bt_att_send(server->att, BT_ATT_OP_ERROR_RSP, rsp_pdu, 4,
> + NULL, NULL, NULL);
> +}
> +
> static void exchange_mtu_cb(uint8_t opcode, const void *pdu,
> uint16_t length, void *user_data)
> {
> @@ -296,6 +528,14 @@ static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
> if (!server->read_by_grp_type_id)
> return false;
>
> + /* Read By Type */
> + server->read_by_type_id = bt_att_register(server->att,
> + BT_ATT_OP_READ_BY_TYPE_REQ,
> + read_by_type_cb,
> + server, NULL);
> + if (!server->read_by_type_id)
> + return false;
> +
> return true;
> }
>
> --
> 2.1.0.rc2.206.gedb03e5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
