* [PATCH BlueZ 5/5] Add unit test for gattrib
From: Michael Janssen @ 2014-10-23 16:33 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Michael Janssen
In-Reply-To: <1414082009-40903-1-git-send-email-jamuraa@chromium.org>
This will ensure that the API behavior of gattrib is preserved.
---
.gitignore | 1 +
Makefile.am | 7 +
unit/test-gattrib.c | 584 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 592 insertions(+)
create mode 100644 unit/test-gattrib.c
diff --git a/.gitignore b/.gitignore
index 97daa9f..164cc97 100644
--- a/.gitignore
+++ b/.gitignore
@@ -121,6 +121,7 @@ unit/test-avdtp
unit/test-avctp
unit/test-avrcp
unit/test-gatt
+unit/test-gattrib
unit/test-*.log
unit/test-*.trs
diff --git a/Makefile.am b/Makefile.am
index 2dfea28..3fddb80 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -377,6 +377,13 @@ unit_test_gatt_SOURCES = unit/test-gatt.c
unit_test_gatt_LDADD = src/libshared-glib.la \
lib/libbluetooth-internal.la @GLIB_LIBS@
+unit_tests += unit/test-gattrib
+
+unit_test_gattrib_SOURCES = unit/test-gattrib.c attrib/gattrib.c $(btio_sources) src/log.h src/log.c
+unit_test_gattrib_LDADD = lib/libbluetooth-internal.la \
+ src/libshared-glib.la \
+ @GLIB_LIBS@ @DBUS_LIBS@ -ldl -lrt
+
if MAINTAINER_MODE
noinst_PROGRAMS += $(unit_tests)
endif
diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c
new file mode 100644
index 0000000..3e14ac1
--- /dev/null
+++ b/unit/test-gattrib.c
@@ -0,0 +1,584 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Google, Inc.
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <inttypes.h>
+#include <string.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+
+#include <glib.h>
+
+#include "src/shared/util.h"
+#include "attrib/gattrib.h"
+#include "src/log.h"
+
+#define DEFAULT_MTU 23
+
+struct test_pdu {
+ bool valid;
+ const uint8_t *data;
+ size_t size;
+};
+
+struct test_data {
+ char *test_name;
+ struct test_pdu *pdu_list;
+};
+
+struct context {
+ GMainLoop *main_loop;
+ guint source;
+ guint process;
+ int fd;
+ unsigned int pdu_offset;
+ const struct test_data *data;
+};
+
+#define data(args...) ((const unsigned char[]) { args })
+
+#define raw_pdu(args...) \
+ { \
+ .valid = true, \
+ .data = data(args), \
+ .size = sizeof(data(args)), \
+ }
+
+#define define_test(name, function, args...) \
+ do { \
+ const struct test_pdu pdus[] = { \
+ args, { } \
+ }; \
+ static struct test_data data; \
+ data.test_name = g_strdup(name); \
+ data.pdu_list = g_malloc(sizeof(pdus)); \
+ memcpy(data.pdu_list, pdus, sizeof(pdus)); \
+ g_test_add_data_func(name, &data, function); \
+ } while (0)
+
+
+static void test_debug(const char *str, void *user_data)
+{
+ const char *prefix = user_data;
+
+ g_print("%s%s\n", prefix, str);
+}
+
+/*
+static void test_free(gconstpointer user_data)
+{
+ const struct test_data *data = user_data;
+
+ g_free(data->test_name);
+ g_free(data->pdu_list);
+}
+
+static gboolean context_quit(gpointer user_data)
+{
+ struct context *context = user_data;
+
+ if (context->process > 0)
+ g_source_remove(context->process);
+
+ g_main_loop_quit(context->main_loop);
+
+ return FALSE;
+}
+
+static gboolean send_pdu(gpointer user_data)
+{
+ struct context *context = user_data;
+ const struct test_pdu *pdu;
+ ssize_t len;
+
+ pdu = &context->data->pdu_list[context->pdu_offset++];
+
+ len = write(context->fd, pdu->data, pdu->size);
+
+ if (g_test_verbose())
+ util_hexdump('<', pdu->data, len, test_debug, "GATT: ");
+
+ g_assert_cmpint(len, ==, pdu->size);
+
+ context->process = 0;
+ return FALSE;
+}
+
+static void context_process(struct context *context)
+{
+ if (!context->data->pdu_list[context->pdu_offset].valid) {
+ context_quit(context);
+ return;
+ }
+
+ context->process = g_idle_add(send_pdu, context);
+}
+
+static gboolean test_handler(GIOChannel *channel, GIOCondition cond,
+ gpointer user_data)
+{
+ struct context *context = user_data;
+ const struct test_pdu *pdu;
+ unsigned char buf[512];
+ ssize_t len;
+ int fd;
+
+ pdu = &context->data->pdu_list[context->pdu_offset++];
+
+ if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+ context->source = 0;
+ g_print("%s: cond %x\n", __func__, cond);
+ return FALSE;
+ }
+
+ fd = g_io_channel_unix_get_fd(channel);
+
+ len = read(fd, buf, sizeof(buf));
+
+ g_assert(len > 0);
+
+ if (g_test_verbose())
+ util_hexdump('>', buf, len, test_debug, "GATT: ");
+
+ g_assert_cmpint(len, ==, pdu->size);
+
+ g_assert(memcmp(buf, pdu->data, pdu->size) == 0);
+
+ context_process(context);
+
+ return TRUE;
+}
+
+static void gatt_debug(const char *str, void *user_data)
+{
+ const char *prefix = user_data;
+
+ g_print("%s%s\n", prefix, str);
+}
+
+static struct context *create_context(uint16_t mtu, gconstpointer data)
+{
+ struct context *context = g_new0(struct context, 1);
+ GIOChannel *channel;
+ int err, sv[2];
+ struct bt_att *att;
+
+ context->main_loop = g_main_loop_new(NULL, FALSE);
+ g_assert(context->main_loop);
+
+ err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
+ g_assert(err == 0);
+
+ att = bt_att_new(sv[0]);
+ g_assert(att);
+
+ context->client = bt_gatt_client_new(att, mtu);
+ g_assert(context->client);
+
+ if (g_test_verbose())
+ bt_gatt_client_set_debug(context->client, gatt_debug, "gatt:",
+ NULL);
+
+ channel = g_io_channel_unix_new(sv[1]);
+
+ g_io_channel_set_close_on_unref(channel, TRUE);
+ g_io_channel_set_encoding(channel, NULL, NULL);
+ g_io_channel_set_buffered(channel, FALSE);
+
+ context->source = g_io_add_watch(channel,
+ G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+ test_handler, context);
+ g_assert(context->source > 0);
+
+ g_io_channel_unref(channel);
+ bt_att_unref(att);
+
+
+ return context;
+}
+
+static void destroy_context(struct context *context)
+{
+ if (context->source > 0)
+ g_source_remove(context->source);
+
+ bt_gatt_client_unref(context->client);
+
+ g_main_loop_unref(context->main_loop);
+
+ test_free(context->data);
+ g_free(context);
+}
+
+static void execute_context(struct context *context)
+{
+ g_main_loop_run(context->main_loop);
+
+ destroy_context(context);
+}
+
+static void test_client(gconstpointer data)
+{
+ struct context *context = create_context(512, data);
+
+ execute_context(context);
+}
+
+*/
+
+static void destroy_canary_increment(gpointer data) {
+ int *canary = data;
+ (*canary)++;
+}
+
+static void test_refcounts(void)
+{
+ int err, sv[2];
+ GAttrib *att, *extra_ref;
+ GIOChannel *channel;
+ int destroy_canary;
+
+ err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
+ g_assert(err == 0);
+
+ channel = g_io_channel_unix_new(sv[1]);
+
+ g_io_channel_set_close_on_unref(channel, TRUE);
+
+ att = g_attrib_new(channel, DEFAULT_MTU);
+
+ destroy_canary = 0;
+
+ g_attrib_set_destroy_function(att, destroy_canary_increment, &destroy_canary);
+
+ extra_ref = g_attrib_ref(att);
+
+ g_assert(extra_ref == att);
+
+ g_attrib_unref(extra_ref);
+
+ g_assert(destroy_canary == 0);
+
+ g_attrib_unref(att);
+
+ g_assert(destroy_canary == 1);
+
+}
+
+static void test_get_channel(void) {
+
+ int err, sv[2];
+ GAttrib *att;
+ GIOChannel *channel, *result;
+
+ err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
+ g_assert(err == 0);
+
+ channel = g_io_channel_unix_new(sv[1]);
+ g_assert(channel != NULL);
+
+ g_io_channel_set_close_on_unref(channel, TRUE);
+
+ att = g_attrib_new(channel, DEFAULT_MTU);
+ g_assert(att != 0);
+
+ result = g_attrib_get_channel(att);
+
+ g_assert(result == channel);
+
+ g_attrib_unref(att);
+}
+
+struct challenge_response {
+ const uint8_t *expect;
+ const size_t expect_len;
+ const uint8_t *respond;
+ const size_t respond_len;
+ bool received;
+};
+
+static gboolean test_client(GIOChannel *channel, GIOCondition cond, gpointer data) {
+ struct challenge_response *cr = data;
+ int fd;
+ uint8_t buf[256];
+ ssize_t len;
+
+ if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+ g_print("%s: no good cond %x\n", __func__, cond);
+ return FALSE;
+ }
+
+ fd = g_io_channel_unix_get_fd(channel);
+
+ len = read(fd, buf, sizeof(buf));
+
+ g_assert(len > 0);
+ g_assert_cmpint(len, ==, cr->expect_len);
+
+ if (g_test_verbose())
+ util_hexdump('>', buf, len, test_debug, "test_client: ");
+
+ cr->received = true;
+
+ if (cr->respond != NULL) {
+ if (g_test_verbose())
+ util_hexdump('<', cr->respond, cr->respond_len, test_debug, "test_client: ");
+ len = write(fd, cr->respond, cr->respond_len);
+
+ g_assert_cmpint(len, ==, cr->respond_len);
+ }
+
+ return TRUE;
+}
+
+struct result_data {
+ guint8 status;
+ const guint8 *pdu;
+ guint16 len;
+ bool done;
+};
+
+static void result_canary(guint8 status, const guint8 *pdu, guint16 len, gpointer data) {
+ struct result_data *result = data;
+ result->status = status;
+ result->pdu = pdu;
+ result->len = len;
+
+ if (g_test_verbose())
+ util_hexdump('<', pdu, len, test_debug, "result_canary: ");
+
+ result->done = true;
+
+}
+
+static void test_send(void) {
+ int err, sv[2];
+ GIOChannel *att_io, *server_io;
+ GAttrib *att;
+ GMainLoop *main_loop;
+ GMainContext *context;
+ struct result_data results;
+
+ main_loop = g_main_loop_new(NULL, FALSE);
+
+ err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
+ g_assert(err == 0);
+
+ att_io = g_io_channel_unix_new(sv[0]);
+ g_assert(att_io != 0);
+
+ g_io_channel_set_close_on_unref(att_io, TRUE);
+
+ att = g_attrib_new(att_io, DEFAULT_MTU);
+ g_assert(att != 0);
+
+ server_io = g_io_channel_unix_new(sv[1]);
+
+ g_io_channel_set_close_on_unref(server_io, TRUE);
+ g_io_channel_set_encoding(server_io, NULL, NULL);
+ g_io_channel_set_buffered(server_io, FALSE);
+
+ do {
+ struct challenge_response data = {
+ .expect = ((unsigned char[]) { 0x02, 0x00, 0x02 }),
+ .expect_len = 3,
+ .respond = ((unsigned char[]) { 0x01, 0x02, 0x03, 0x04 }),
+ .respond_len = 4,
+ .received = false,
+ };
+
+ g_io_add_watch(server_io, G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+ test_client, &data);
+
+ results.done = false;
+
+ g_attrib_send(att, 0, data.expect, data.expect_len, result_canary,
+ (gpointer) &results, NULL);
+
+ // Spin the main loop until we are ready.
+ context = g_main_loop_get_context(main_loop);
+ do {
+ g_main_context_iteration(context, TRUE);
+ } while (!results.done);
+ } while (0);
+
+ g_io_channel_unref(server_io);
+
+ g_attrib_unref(att);
+}
+
+static void test_cancel(void) {
+ int err, sv[2];
+ GIOChannel *att_io, *server_io;
+ GAttrib *att;
+ guint event_id;
+ GMainLoop *main_loop;
+ GMainContext *context;
+ gboolean canceled;
+ struct result_data results;
+
+ main_loop = g_main_loop_new(NULL, FALSE);
+
+ err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
+ g_assert(err == 0);
+
+ att_io = g_io_channel_unix_new(sv[0]);
+ g_assert(att_io != 0);
+
+ g_io_channel_set_close_on_unref(att_io, TRUE);
+
+ att = g_attrib_new(att_io, DEFAULT_MTU);
+ g_assert(att != 0);
+
+ server_io = g_io_channel_unix_new(sv[1]);
+
+ do {
+ struct challenge_response data = {
+ .expect = ((unsigned char[]) { 0x02, 0x00, 0x02 }),
+ .expect_len = 3,
+ .respond = ((unsigned char[]) { 0x01, 0x02, 0x03, 0x04 }),
+ .respond_len = 4,
+ .received = false,
+ };
+
+ g_io_add_watch(server_io,
+ G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+ test_client, &data);
+
+ results.done = false;
+ event_id = g_attrib_send(att, 0, data.expect,
+ data.expect_len, result_canary,
+ &results, NULL);
+
+ /* Spin the main loop until we receive the first message */
+ context = g_main_loop_get_context(main_loop);
+ do {
+ g_main_context_iteration(context, FALSE);
+ } while (!data.received);
+
+ canceled = g_attrib_cancel(att, event_id);
+ g_assert(canceled);
+
+ /*
+ * Spin the main loop until all events are processed,
+ * no result should be called */
+ do {
+ g_main_context_iteration(context, TRUE);
+ } while (g_main_context_pending(context));
+
+ g_assert(!results.done);
+
+
+ results.done = false;
+ data.received = false;
+ event_id = g_attrib_send(att, 0, data.expect, data.expect_len, result_canary,
+ &results, NULL);
+
+ canceled = g_attrib_cancel(att, event_id);
+ g_assert(canceled);
+
+ /*
+ * Spin the main loop until all events are processed,
+ * the message should never be delivered.
+ */
+ do {
+ g_main_context_iteration(context, TRUE);
+ } while (g_main_context_pending(context));
+
+ g_assert(!data.received);
+ g_assert(!results.done);
+
+ /* Invalid ID */
+ canceled = g_attrib_cancel(att, 42);
+ g_assert(!canceled);
+ } while (0);
+
+ g_io_channel_unref(server_io);
+
+ g_attrib_unref(att);
+}
+
+static void test_register(void) {
+ /* TODO */
+}
+
+static void test_buffers(void) {
+ int err, sv[2];
+ GIOChannel *att_io;
+ GAttrib *att;
+ size_t buflen;
+ uint8_t *buf;
+ gboolean success;
+
+ err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
+ g_assert(err == 0);
+
+ att_io = g_io_channel_unix_new(sv[0]);
+ g_assert(att_io != 0);
+
+ g_io_channel_set_close_on_unref(att_io, TRUE);
+
+ att = g_attrib_new(att_io, DEFAULT_MTU);
+ g_assert(att != 0);
+
+ buf = g_attrib_get_buffer(att, &buflen);
+ g_assert(buf != 0);
+ g_assert_cmpint(buflen, ==, DEFAULT_MTU);
+
+ success = g_attrib_set_mtu(att, 5);
+ g_assert(!success);
+
+ success = g_attrib_set_mtu(att, 255);
+ g_assert(success);
+
+ buf = g_attrib_get_buffer(att, &buflen);
+ g_assert(buf != 0);
+ g_assert_cmpint(buflen, ==, 255);
+
+ g_attrib_unref(att);
+}
+
+int main(int argc, char *argv[])
+{
+ g_test_init(&argc, &argv, NULL);
+
+ __btd_log_init("*", 0);
+
+ /*
+ * Test the GAttrib API behavior
+ */
+ g_test_add_func("/gattrib/refcount", test_refcounts);
+ g_test_add_func("/gattrib/get_channel", test_get_channel);
+ g_test_add_func("/gattrib/send", test_send);
+ g_test_add_func("/gattrib/cancel", test_cancel);
+ g_test_add_func("/gattrib/register", test_register);
+ g_test_add_func("/gattrib/buffers", test_buffers);
+
+ return g_test_run();
+}
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH BlueZ 4/5] attrib: remove g_attrib_is_encrypted
From: Michael Janssen @ 2014-10-23 16:33 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Michael Janssen
In-Reply-To: <1414082009-40903-1-git-send-email-jamuraa@chromium.org>
This function is only used in one place and encryption is the
responsibility of the channel, not the attribute.
---
attrib/gattrib.c | 12 ------------
attrib/gattrib.h | 2 --
src/attrib-server.c | 9 +++++++--
3 files changed, 7 insertions(+), 16 deletions(-)
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index a6511a2..a65d1ca 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -690,18 +690,6 @@ static int event_cmp_by_id(gconstpointer a, gconstpointer b)
return evt->id - id;
}
-gboolean g_attrib_is_encrypted(GAttrib *attrib)
-{
- BtIOSecLevel sec_level;
-
- if (!bt_io_get(attrib->io, NULL,
- BT_IO_OPT_SEC_LEVEL, &sec_level,
- BT_IO_OPT_INVALID))
- return FALSE;
-
- return sec_level > BT_IO_SEC_LOW;
-}
-
gboolean g_attrib_unregister(GAttrib *attrib, guint id)
{
struct event *evt;
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index 99b8b37..1557b99 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -62,8 +62,6 @@ guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle,
GAttribNotifyFunc func, gpointer user_data,
GDestroyNotify notify);
-gboolean g_attrib_is_encrypted(GAttrib *attrib);
-
uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len);
gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu);
diff --git a/src/attrib-server.c b/src/attrib-server.c
index e65fff2..1ccfc65 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -375,12 +375,17 @@ static struct attribute *attrib_db_add_new(struct gatt_server *server,
static uint8_t att_check_reqs(struct gatt_channel *channel, uint8_t opcode,
int reqs)
{
+ BtIOSecLevel sec_level;
+ GIOChannel *io = g_attrib_get_channel(channel->attrib);
+
/* FIXME: currently, it is assumed an encrypted link is enough for
* authentication. This will allow to enable the SMP negotiation once
* it is on upstream kernel. High security level should be mapped
* to authentication and medium to encryption permission. */
- if (!channel->encrypted)
- channel->encrypted = g_attrib_is_encrypted(channel->attrib);
+ if (!channel->encrypted &&
+ bt_io_get(io, NULL, BT_IO_OPT_SEC_LEVEL, &sec_level,
+ BT_IO_OPT_INVALID))
+ channel->encrypted = sec_level > BT_IO_SEC_LOW;
if (reqs == ATT_AUTHENTICATION && !channel->encrypted)
return ATT_ECODE_AUTHENTICATION;
else if (reqs == ATT_AUTHORIZATION)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH BlueZ 3/5] attrib: Add mtu argument to g_attrib_new
From: Michael Janssen @ 2014-10-23 16:33 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Michael Janssen
In-Reply-To: <1414082009-40903-1-git-send-email-jamuraa@chromium.org>
Instead of using the default MTU, use one passed
in by the user, and detect it from the channel when
it is created.
---
attrib/gattrib.c | 10 +++-------
attrib/gattrib.h | 2 +-
attrib/gatttool.c | 17 ++++++++++++++++-
attrib/interactive.c | 17 ++++++++++++++++-
src/device.c | 11 ++++++++++-
5 files changed, 46 insertions(+), 11 deletions(-)
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index fa41ade..a6511a2 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -473,11 +473,9 @@ done:
return TRUE;
}
-GAttrib *g_attrib_new(GIOChannel *io)
+GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu)
{
struct _GAttrib *attrib;
- uint16_t att_mtu;
- uint16_t cid;
g_io_channel_set_encoding(io, NULL, NULL);
g_io_channel_set_buffered(io, FALSE);
@@ -486,10 +484,8 @@ GAttrib *g_attrib_new(GIOChannel *io)
if (attrib == NULL)
return NULL;
- att_mtu = ATT_DEFAULT_LE_MTU;
-
- attrib->buf = g_malloc0(att_mtu);
- attrib->buflen = att_mtu;
+ attrib->buf = g_malloc0(mtu);
+ attrib->buflen = mtu;
attrib->io = g_io_channel_ref(io);
attrib->requests = g_queue_new();
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index 18df2ad..99b8b37 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -42,7 +42,7 @@ typedef void (*GAttribDebugFunc)(const char *str, gpointer user_data);
typedef void (*GAttribNotifyFunc)(const guint8 *pdu, guint16 len,
gpointer user_data);
-GAttrib *g_attrib_new(GIOChannel *io);
+GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu);
GAttrib *g_attrib_ref(GAttrib *attrib);
void g_attrib_unref(GAttrib *attrib);
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index db5da62..8f92d62 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -123,6 +123,9 @@ static gboolean listen_start(gpointer user_data)
static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)
{
GAttrib *attrib;
+ uint16_t mtu;
+ uint16_t cid;
+ GError *gerr = NULL;
if (err) {
g_printerr("%s\n", err->message);
@@ -130,7 +133,19 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)
g_main_loop_quit(event_loop);
}
- attrib = g_attrib_new(io);
+ bt_io_get(io, &gerr, BT_IO_OPT_IMTU, &mtu,
+ BT_IO_OPT_CID, &cid, BT_IO_OPT_INVALID);
+
+ if (gerr) {
+ g_printerr("Can't detect MTU, using default: %s", gerr->message);
+ g_error_free(gerr);
+ mtu = ATT_DEFAULT_LE_MTU;
+ }
+
+ if (cid == ATT_CID)
+ mtu = ATT_DEFAULT_LE_MTU;
+
+ attrib = g_attrib_new(io, mtu);
if (opt_listen)
g_idle_add(listen_start, attrib);
diff --git a/attrib/interactive.c b/attrib/interactive.c
index 08f39f7..7911ba5 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -150,13 +150,28 @@ static void events_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)
{
+ uint16_t mtu;
+ uint16_t cid;
+
if (err) {
set_state(STATE_DISCONNECTED);
error("%s\n", err->message);
return;
}
- attrib = g_attrib_new(iochannel);
+ bt_io_get(io, &err, BT_IO_OPT_IMTU, &mtu,
+ BT_IO_OPT_CID, &cid, BT_IO_OPT_INVALID);
+
+ if (err) {
+ g_printerr("Can't detect MTU, using default: %s", err->message);
+ g_error_free(err);
+ mtu = ATT_DEFAULT_LE_MTU;
+ }
+
+ if (cid == ATT_CID)
+ mtu = ATT_DEFAULT_LE_MTU;
+
+ attrib = g_attrib_new(iochannel, mtu);
g_attrib_register(attrib, ATT_OP_HANDLE_NOTIFY, GATTRIB_ALL_HANDLES,
events_handler, attrib, NULL);
g_attrib_register(attrib, ATT_OP_HANDLE_IND, GATTRIB_ALL_HANDLES,
diff --git a/src/device.c b/src/device.c
index 875a5c5..0925951 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3627,11 +3627,17 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
GError *gerr = NULL;
GAttrib *attrib;
BtIOSecLevel sec_level;
+ uint16_t mtu;
+ uint16_t cid;
bt_io_get(io, &gerr, BT_IO_OPT_SEC_LEVEL, &sec_level,
+ BT_IO_OPT_IMTU, &mtu,
+ BT_IO_OPT_CID, &cid,
BT_IO_OPT_INVALID);
+
if (gerr) {
error("bt_io_get: %s", gerr->message);
+ mtu = ATT_DEFAULT_LE_MTU;
g_error_free(gerr);
return false;
}
@@ -3649,7 +3655,10 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
}
}
- attrib = g_attrib_new(io);
+ if (cid == ATT_CID)
+ mtu = ATT_DEFAULT_LE_MTU;
+
+ attrib = g_attrib_new(io, mtu);
if (!attrib) {
error("Unable to create new GAttrib instance");
return false;
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH BlueZ 2/5] attrib: Remove MTU-probing code
From: Michael Janssen @ 2014-10-23 16:33 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Michael Janssen
In-Reply-To: <1414082009-40903-1-git-send-email-jamuraa@chromium.org>
Probing for the MTU using bt_io is problematic for
testing because you cannot impersonate AF_BLUETOOTH sockets
with a socketpair.
---
attrib/gattrib.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 71d1cef..fa41ade 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -476,27 +476,17 @@ done:
GAttrib *g_attrib_new(GIOChannel *io)
{
struct _GAttrib *attrib;
- uint16_t imtu;
uint16_t att_mtu;
uint16_t cid;
- GError *gerr = NULL;
g_io_channel_set_encoding(io, NULL, NULL);
g_io_channel_set_buffered(io, FALSE);
- bt_io_get(io, &gerr, BT_IO_OPT_IMTU, &imtu,
- BT_IO_OPT_CID, &cid, BT_IO_OPT_INVALID);
- if (gerr) {
- error("%s", gerr->message);
- g_error_free(gerr);
- return NULL;
- }
-
attrib = g_try_new0(struct _GAttrib, 1);
if (attrib == NULL)
return NULL;
- att_mtu = (cid == ATT_CID) ? ATT_DEFAULT_LE_MTU : imtu;
+ att_mtu = ATT_DEFAULT_LE_MTU;
attrib->buf = g_malloc0(att_mtu);
attrib->buflen = att_mtu;
@@ -651,7 +641,6 @@ gboolean g_attrib_cancel_all(GAttrib *attrib)
return ret;
}
-
uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len)
{
if (len == NULL)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH BlueZ 1/5] Remove unused g_attrib_set_debug function
From: Michael Janssen @ 2014-10-23 16:33 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Michael Janssen
In-Reply-To: <1414082009-40903-1-git-send-email-jamuraa@chromium.org>
This function is not used, and also not implemented.
---
attrib/gattrib.c | 5 -----
attrib/gattrib.h | 3 ---
2 files changed, 8 deletions(-)
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 912dffb..71d1cef 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -651,11 +651,6 @@ gboolean g_attrib_cancel_all(GAttrib *attrib)
return ret;
}
-gboolean g_attrib_set_debug(GAttrib *attrib,
- GAttribDebugFunc func, gpointer user_data)
-{
- return TRUE;
-}
uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len)
{
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index 3fe92c7..18df2ad 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -58,9 +58,6 @@ guint g_attrib_send(GAttrib *attrib, guint id, const guint8 *pdu, guint16 len,
gboolean g_attrib_cancel(GAttrib *attrib, guint id);
gboolean g_attrib_cancel_all(GAttrib *attrib);
-gboolean g_attrib_set_debug(GAttrib *attrib,
- GAttribDebugFunc func, gpointer user_data);
-
guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle,
GAttribNotifyFunc func, gpointer user_data,
GDestroyNotify notify);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH BlueZ 0/5] Unit tests for GAttrib
From: Michael Janssen @ 2014-10-23 16:33 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Michael Janssen
Since we want to shim GAttrib for a smooth transition to bt_att
profiles, this patch cleans up the API and adds unit tests for
GAttrib functions.
This adds tests for all but the register / unregister functions, which
I will send in a separate patch since they are slightly more complicated due
to catchalls.
Michael Janssen (5):
Remove unused g_attrib_set_debug function
attrib: Remove MTU-probing code
attrib: Add mtu argument to g_attrib_new
attrib: remove g_attrib_is_encrypted
Add unit test for gattrib
.gitignore | 1 +
Makefile.am | 7 +
attrib/gattrib.c | 38 +---
attrib/gattrib.h | 7 +-
attrib/gatttool.c | 17 +-
attrib/interactive.c | 17 +-
src/attrib-server.c | 9 +-
src/device.c | 11 +-
unit/test-gattrib.c | 584 +++++++++++++++++++++++++++++++++++++++++++++++++++
9 files changed, 645 insertions(+), 46 deletions(-)
create mode 100644 unit/test-gattrib.c
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply
* Re: [PATCH 1/1 net-next] Bluetooth: fix shadow warning in hci_disconnect()
From: Marcel Holtmann @ 2014-10-23 16:29 UTC (permalink / raw)
To: Fabian Frederick
Cc: linux-kernel, Gustavo F. Padovan, Johan Hedberg, David S. Miller,
BlueZ development, Network Development
In-Reply-To: <1414081365-6461-1-git-send-email-fabf@skynet.be>
Hi Fabian,
> use cpr for hci_cp_read_clock_offset instead of cp
> (already defined above).
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> net/bluetooth/hci_conn.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index b9517bd..6151e09 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -141,10 +141,10 @@ int hci_disconnect(struct hci_conn *conn, __u8 reason)
> */
> if (conn->type == ACL_LINK && conn->role == HCI_ROLE_MASTER) {
> struct hci_dev *hdev = conn->hdev;
> - struct hci_cp_read_clock_offset cp;
> + struct hci_cp_read_clock_offset cpr;
>
> - cp.handle = cpu_to_le16(conn->handle);
> - hci_send_cmd(hdev, HCI_OP_READ_CLOCK_OFFSET, sizeof(cp), &cp);
> + cpr.handle = cpu_to_le16(conn->handle);
> + hci_send_cmd(hdev, HCI_OP_READ_CLOCK_OFFSET, sizeof(cpr), &cpr);
valid change, but I do not like cpr as variable name. We need to come up with a better one. There are other places in the code where we had the same thing. Please have a look there.
Regards
Marcel
^ permalink raw reply
* [PATCH 1/1 net-next] Bluetooth: fix shadow warning in hci_disconnect()
From: Fabian Frederick @ 2014-10-23 16:22 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, Marcel Holtmann, Gustavo Padovan, Johan Hedberg,
David S. Miller, linux-bluetooth, netdev
use cpr for hci_cp_read_clock_offset instead of cp
(already defined above).
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/bluetooth/hci_conn.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b9517bd..6151e09 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -141,10 +141,10 @@ int hci_disconnect(struct hci_conn *conn, __u8 reason)
*/
if (conn->type == ACL_LINK && conn->role == HCI_ROLE_MASTER) {
struct hci_dev *hdev = conn->hdev;
- struct hci_cp_read_clock_offset cp;
+ struct hci_cp_read_clock_offset cpr;
- cp.handle = cpu_to_le16(conn->handle);
- hci_send_cmd(hdev, HCI_OP_READ_CLOCK_OFFSET, sizeof(cp), &cp);
+ cpr.handle = cpu_to_le16(conn->handle);
+ hci_send_cmd(hdev, HCI_OP_READ_CLOCK_OFFSET, sizeof(cpr), &cpr);
}
conn->state = BT_DISCONN;
--
1.9.3
^ permalink raw reply related
* Re: [PATCH v4 08/11] unit/test-hfp: Add init test for HFP HF
From: Lukasz Rymanowski @ 2014-10-23 15:00 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <44190135.DuM4UiHnJI@uw000953>
Hi Szymon,
On 22 October 2014 13:00, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi Łukasz,
>
> On Friday 10 of October 2014 01:50:08 Lukasz Rymanowski wrote:
>> This patch adds basic infrastruction for HFP HF test plus
>> init test.
>>
>> It also moves send_pdu function in the file so it can be used by
>> test_hf_handler
>> ---
>> unit/test-hfp.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++----------
>> 1 file changed, 84 insertions(+), 18 deletions(-)
>>
>> diff --git a/unit/test-hfp.c b/unit/test-hfp.c
>> index 4b3473b..274ee55 100644
>> --- a/unit/test-hfp.c
>> +++ b/unit/test-hfp.c
>> @@ -36,6 +36,7 @@ struct context {
>> int fd_server;
>> int fd_client;
>> struct hfp_gw *hfp;
>> + struct hfp_hf *hfp_hf;
>> const struct test_data *data;
>> unsigned int pdu_offset;
>> };
>> @@ -52,6 +53,8 @@ struct test_data {
>> char *test_name;
>> struct test_pdu *pdu_list;
>> hfp_result_func_t result_func;
>> + hfp_response_func_t response_func;
>> + hfp_hf_result_func_t hf_result_func;
>> GIOFunc test_handler;
>> };
>>
>> @@ -99,6 +102,22 @@ struct test_data {
>> data.test_handler = test_handler; \
>> } while (0)
>>
>> +#define define_hf_test(name, function, result_func, response_function, \
>> + args...)\
>> + do { \
>> + const struct test_pdu pdus[] = { \
>> + args, { } \
>> + }; \
>> + static struct test_data data; \
>> + data.test_name = g_strdup(name); \
>> + data.pdu_list = g_malloc(sizeof(pdus)); \
>> + data.hf_result_func = result_func; \
>> + data.response_func = response_function; \
>> + memcpy(data.pdu_list, pdus, sizeof(pdus)); \
>> + g_test_add_data_func(name, &data, function); \
>> + data.test_handler = test_hf_handler; \
>> + } while (0)
>> +
>> static void context_quit(struct context *context)
>> {
>> g_main_loop_quit(context->main_loop);
>> @@ -128,6 +147,52 @@ static gboolean test_handler(GIOChannel *channel, GIOCondition cond,
>> return FALSE;
>> }
>>
>> +static gboolean send_pdu(gpointer user_data)
>> +{
>> + struct context *context = user_data;
>> + const struct test_pdu *pdu;
>> + ssize_t len;
>> +
>> + pdu = &context->data->pdu_list[context->pdu_offset++];
>> +
>> + if (pdu && !pdu->valid)
>> + return FALSE;
>> +
>> + len = write(context->fd_server, pdu->data, pdu->size);
>> + g_assert_cmpint(len, ==, pdu->size);
>> +
>> + pdu = &context->data->pdu_list[context->pdu_offset];
>> + if (pdu->fragmented)
>> + g_idle_add(send_pdu, context);
>> +
>> + return FALSE;
>> +}
>> +
>> +static gboolean test_hf_handler(GIOChannel *channel, GIOCondition cond,
>> + gpointer user_data)
>> +{
>> + struct context *context = user_data;
>> + gchar buf[60];
>> + gsize bytes_read;
>> + GError *error = NULL;
>> +
>> + if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL))
>> + goto done;
>> +
>> + /* dummy read */
>> + g_io_channel_read_chars(channel, buf, 60, &bytes_read, &error);
>> +
>> + send_pdu(context);
>> +
>> + return TRUE;
>> +
>> +done:
>> + context_quit(context);
>> + context->watch_id = 0;
>> +
>> + return FALSE;
>> +}
>> +
>> static void cmd_handler(const char *command, void *user_data)
>> {
>> struct context *context = user_data;
>> @@ -203,6 +268,9 @@ static void execute_context(struct context *context)
>> if (context->hfp)
>> hfp_gw_unref(context->hfp);
>>
>> + if (context->hfp_hf)
>> + hfp_hf_unref(context->hfp_hf);
>> +
>> g_free(context);
>> }
>>
>> @@ -275,24 +343,6 @@ static void test_register(gconstpointer data)
>> execute_context(context);
>> }
>>
>> -static gboolean send_pdu(gpointer user_data)
>> -{
>> - struct context *context = user_data;
>> - const struct test_pdu *pdu;
>> - ssize_t len;
>> -
>> - pdu = &context->data->pdu_list[context->pdu_offset++];
>> -
>> - len = write(context->fd_server, pdu->data, pdu->size);
>> - g_assert_cmpint(len, ==, pdu->size);
>> -
>> - pdu = &context->data->pdu_list[context->pdu_offset];
>> - if (pdu->fragmented)
>> - g_idle_add(send_pdu, context);
>> -
>> - return FALSE;
>> -}
>> -
>> static void test_fragmented(gconstpointer data)
>> {
>> struct context *context = create_context(data);
>> @@ -404,6 +454,20 @@ static void check_string_2(struct hfp_gw_result *result,
>> hfp_gw_send_result(context->hfp, HFP_RESULT_ERROR);
>> }
>>
>> +static void test_hf_init(gconstpointer data)
>> +{
>> + struct context *context = create_context(data);
>> +
>> + context->hfp_hf = hfp_hf_new(context->fd_client);
>> + g_assert(context->hfp_hf);
>> + g_assert(hfp_hf_set_close_on_unref(context->hfp_hf, true));
>> +
>> + hfp_hf_unref(context->hfp_hf);
>> + context->hfp_hf = NULL;
>> +
>> + execute_context(context);
>> +}
>> +
>> int main(int argc, char *argv[])
>> {
>> g_test_init(&argc, &argv, NULL);
>> @@ -473,5 +537,7 @@ int main(int argc, char *argv[])
>> raw_pdu('\r'),
>> data_end());
>>
>> + define_hf_test("/hfp/test_init", test_hf_init, NULL, NULL, data_end());
>
> I'd prefer if all hfp_hf tests were prefixed like this:
> "/hfp_hf/test_foo"
>
> This will allow to avoid doubling tests name like this one (there is already
> /hfp/test_init test).
>
OK
>> +
>> return g_test_run();
>> }
>>
>
> --
> Best regards,
> Szymon Janc
\Łukasz
^ permalink raw reply
* Re: [PATCH v4 06/11] shared/hfp: Add send AT command API for HFP HF
From: Lukasz Rymanowski @ 2014-10-23 15:00 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <2209288.OLlArcbHcy@uw000953>
Hi Szymon,
On 22 October 2014 13:00, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi Łukasz,
>
> On Friday 10 of October 2014 01:50:06 Lukasz Rymanowski wrote:
>> This patch adds handling send and response of AT command.
>> Note that we always wait for AT command response before sending next
>> command, however user can fill hfp_hf with more than one command.
>> All the commands are queued and send one by one.
>> ---
>> src/shared/hfp.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> src/shared/hfp.h | 8 +++
>> 2 files changed, 183 insertions(+)
>>
>> diff --git a/src/shared/hfp.c b/src/shared/hfp.c
>> index 5179092..8bebe97 100644
>> --- a/src/shared/hfp.c
>> +++ b/src/shared/hfp.c
>> @@ -70,6 +70,9 @@ struct hfp_hf {
>> struct ringbuf *read_buf;
>> struct ringbuf *write_buf;
>>
>> + bool writer_active;
>> + struct queue *cmd_queue;
>> +
>> struct queue *event_handlers;
>>
>> hfp_debug_func_t debug_callback;
>> @@ -101,6 +104,14 @@ struct hfp_hf_result {
>> unsigned int offset;
>> };
>>
>> +struct cmd_response {
>> + char *prefix;
>> + hfp_response_func_t resp_cb;
>> + struct hfp_hf_result *response;
>> + char *resp_data;
>> + void *user_data;
>> +};
>> +
>> struct event_handler {
>> char *prefix;
>> void *user_data;
>> @@ -868,17 +879,103 @@ static void destroy_event_handler(void *data)
>> free(handler);
>> }
>>
>> +static bool hf_can_write_data(struct io *io, void *user_data)
>> +{
>> + struct hfp_hf *hfp = user_data;
>> + ssize_t bytes_written;
>> +
>> + bytes_written = ringbuf_write(hfp->write_buf, hfp->fd);
>> + if (bytes_written < 0)
>> + return false;
>> +
>> + if (ringbuf_len(hfp->write_buf) > 0)
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +static void hf_write_watch_destroy(void *user_data)
>> +{
>> + struct hfp_hf *hfp = user_data;
>> +
>> + hfp->writer_active = false;
>> +}
>> +
>> static void hf_skip_whitespace(struct hfp_hf_result *result)
>> {
>> while (result->data[result->offset] == ' ')
>> result->offset++;
>> }
>>
>> +static bool is_response(const char *prefix, enum hfp_result *result)
>> +{
>> + if (strcmp(prefix, "OK") == 0) {
>> + *result = HFP_RESULT_OK;
>> + return true;
>> + }
>> +
>> + if (strcmp(prefix, "ERROR") == 0) {
>> + *result = HFP_RESULT_ERROR;
>> + return true;
>> + }
>> +
>> + if (strcmp(prefix, "NO CARRIER") == 0) {
>> + *result = HFP_RESULT_NO_CARRIER;
>> + return true;
>> + }
>> +
>> + if (strcmp(prefix, "CONNECT") == 0) {
>
> Shouldn't this be "BUSY"?
>
>> + *result = HFP_RESULT_CONNECT;
>
> And this enum value looks bogus to me.
> I couldn't find it in HFP nor HSP spec. Probably leftover from AT spec.
> I'd just handle what is defined in HFP spec here.
This comes from AT spec. I based on hfp_result enum but indeed this is
not needed. Will fix that and hfp_result enum. Agree that we need only
HFP/HSP related result here.
>
>> + return true;
>> + }
>> +
>> + if (strcmp(prefix, "NO ANSWER") == 0) {
>> + *result = HFP_RESULT_NO_ANSWER;
>> + return true;
>> + }
>> +
>> + if (strcmp(prefix, "DELAYED") == 0) {
>> + *result = HFP_RESULT_DELAYED;
>> + return true;
>> + }
>> +
>> + if (strcmp(prefix, "BLACKLISTED") == 0) {
>> + *result = HFP_RESULT_BLACKLISTED;
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static void hf_wakeup_writer(struct hfp_hf *hfp)
>> +{
>> + if (hfp->writer_active)
>> + return;
>> +
>> + if (!ringbuf_len(hfp->write_buf))
>> + return;
>> +
>> + if (!io_set_write_handler(hfp->io, hf_can_write_data,
>> + hfp, hf_write_watch_destroy))
>> + return;
>> +
>> + hfp->writer_active = true;
>> +}
>> +
>> +static void destroy_cmd_response(void *data)
>> +{
>> + struct cmd_response *cmd = data;
>> +
>> + free(cmd->prefix);
>> + free(cmd);
>> +}
>> +
>> static void hf_call_prefix_handler(struct hfp_hf *hfp, const char *data)
>> {
>> struct event_handler *handler;
>> const char *separators = ";:\0";
>> struct hfp_hf_result result_data;
>> + enum hfp_result result;
>> char lookup_prefix[18];
>> uint8_t pref_len = 0;
>> const char *prefix;
>> @@ -904,6 +1001,22 @@ static void hf_call_prefix_handler(struct hfp_hf *hfp, const char *data)
>> lookup_prefix[pref_len] = '\0';
>> result_data.offset += pref_len + 1;
>>
>> + if (is_response(lookup_prefix, &result)) {
>> + struct cmd_response *cmd;
>> +
>> + cmd = queue_peek_head(hfp->cmd_queue);
>> + if (!cmd)
>> + return;
>> +
>> + cmd->resp_cb(cmd->prefix, result, cmd->user_data);
>> +
>> + queue_remove(hfp->cmd_queue, cmd);
>> + destroy_cmd_response(cmd);
>> +
>> + hf_wakeup_writer(hfp);
>> + return;
>> + }
>> +
>> handler = queue_find(hfp->event_handlers, match_handler_event_prefix,
>> lookup_prefix);
>> if (!handler)
>> @@ -1032,6 +1145,18 @@ struct hfp_hf *hfp_hf_new(int fd)
>> return NULL;
>> }
>>
>> + hfp->cmd_queue = queue_new();
>> + if (!hfp->cmd_queue) {
>> + io_destroy(hfp->io);
>> + ringbuf_free(hfp->write_buf);
>> + ringbuf_free(hfp->read_buf);
>> + queue_destroy(hfp->event_handlers, NULL);
>> + free(hfp);
>> + return NULL;
>> + }
>> +
>> + hfp->writer_active = false;
>> +
>> if (!io_set_read_handler(hfp->io, hf_can_read_data, hfp,
>> read_watch_destroy)) {
>> queue_destroy(hfp->event_handlers,
>> @@ -1083,6 +1208,9 @@ void hfp_hf_unref(struct hfp_hf *hfp)
>> queue_destroy(hfp->event_handlers, destroy_event_handler);
>> hfp->event_handlers = NULL;
>>
>> + queue_destroy(hfp->cmd_queue, destroy_cmd_response);
>> + hfp->cmd_queue = NULL;
>> +
>> if (!hfp->in_disconnect) {
>> free(hfp);
>> return;
>> @@ -1142,6 +1270,53 @@ bool hfp_hf_set_close_on_unref(struct hfp_hf *hfp, bool do_close)
>> return true;
>> }
>>
>> +bool hfp_hf_send_command(struct hfp_hf *hfp, hfp_response_func_t resp_cb,
>> + void *user_data, const char *format, ...)
>> +{
>> + va_list ap;
>> + char *fmt;
>> + int len;
>> + const char *separators = ";?=\0";
>> + uint8_t prefix_len;
>> + struct cmd_response *cmd;
>> +
>> + if (!hfp || !format || !resp_cb)
>> + return false;
>> +
>> + if (asprintf(&fmt, "%s\r", format) < 0)
>> + return false;
>> +
>> + cmd = new0(struct cmd_response, 1);
>> + if (!cmd)
>> + return false;
>> +
>> + va_start(ap, format);
>> + len = ringbuf_vprintf(hfp->write_buf, fmt, ap);
>> + va_end(ap);
>> +
>> + free(fmt);
>> +
>> + if (len < 0) {
>> + free(cmd);
>> + return false;
>> + }
>> +
>> + prefix_len = strcspn(format, separators);
>
> I'd explore possibility of passing prefix as separate argument to the
> function to avoid need of this extra parsing. Also do we really need this
> prefix at all? We should not have more than one pending AT command anyway.
Well this is some leftover from my previous patches where I based on
it (had single function for command complete where I check that and
move with SLC connection setup) Now It is not needed in so will
basically remove it.
>
>> + cmd->prefix = strndup(format, prefix_len);
>> + cmd->resp_cb = resp_cb;
>> + cmd->user_data = user_data;
>> +
>> + if (!queue_push_tail(hfp->cmd_queue, cmd)) {
>> + ringbuf_drain(hfp->write_buf, len);
>> + free(cmd);
>> + return false;
>> + }
>> +
>> + hf_wakeup_writer(hfp);
>> +
>> + return true;
>> +}
>> +
>> bool hfp_hf_register(struct hfp_hf *hfp, hfp_hf_result_func_t callback,
>> const char *prefix,
>> void *user_data,
>> diff --git a/src/shared/hfp.h b/src/shared/hfp.h
>> index 85037b1..5ee3017 100644
>> --- a/src/shared/hfp.h
>> +++ b/src/shared/hfp.h
>> @@ -32,6 +32,8 @@ enum hfp_result {
>> HFP_RESULT_NO_DIALTONE = 6,
>> HFP_RESULT_BUSY = 7,
>> HFP_RESULT_NO_ANSWER = 8,
>> + HFP_RESULT_DELAYED = 9,
>> + HFP_RESULT_BLACKLISTED = 10,
>> };
>>
>> enum hfp_error {
>> @@ -83,6 +85,10 @@ typedef void (*hfp_command_func_t)(const char *command, void *user_data);
>>
>> typedef void (*hfp_disconnect_func_t)(void *user_data);
>>
>> +typedef void (*hfp_response_func_t)(const char *prefix,
>> + enum hfp_result result,
>> + void *user_data);
>> +
>> struct hfp_gw;
>> struct hfp_hf;
>>
>> @@ -146,3 +152,5 @@ bool hfp_hf_register(struct hfp_hf *hfp, hfp_hf_result_func_t callback,
>> const char *prefix, void *user_data,
>> hfp_destroy_func_t destroy);
>> bool hfp_hf_unregister(struct hfp_hf *hfp, const char *prefix);
>> +bool hfp_hf_send_command(struct hfp_hf *hfp, hfp_response_func_t resp_cb,
>> + void *user_data, const char *format, ...);
>>
>
> --
> Best regards,
> Szymon Janc
BR
Lukasz
^ permalink raw reply
* Re: [PATCH v4 01/11] shared/hfp: Add support for HFP HF
From: Lukasz Rymanowski @ 2014-10-23 15:00 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <2009037.IXf9F6m0Mk@uw000953>
Hi Szymon,
On 22 October 2014 13:00, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi Łukasz,
>
> On Friday 10 of October 2014 01:50:01 Lukasz Rymanowski wrote:
>> This patch add struct hfp_hf plus fuctions to create an instance ref and
>> unref. This code based on existing hfp_gw
>> ---
>> src/shared/hfp.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> src/shared/hfp.h | 6 ++++
>> 2 files changed, 98 insertions(+)
>>
>> diff --git a/src/shared/hfp.c b/src/shared/hfp.c
>> index efc981f..dbd049a 100644
>> --- a/src/shared/hfp.c
>> +++ b/src/shared/hfp.c
>> @@ -62,6 +62,18 @@ struct hfp_gw {
>> bool destroyed;
>> };
>>
>> +struct hfp_hf {
>> + int ref_count;
>> + int fd;
>> + bool close_on_unref;
>> + struct io *io;
>> + struct ringbuf *read_buf;
>> + struct ringbuf *write_buf;
>> +
>> + bool in_disconnect;
>> + bool destroyed;
>> +};
>> +
>> struct cmd_handler {
>> char *prefix;
>> void *user_data;
>> @@ -807,3 +819,83 @@ bool hfp_gw_disconnect(struct hfp_gw *hfp)
>>
>> return io_shutdown(hfp->io);
>> }
>> +
>> +struct hfp_hf *hfp_hf_new(int fd)
>> +{
>> + struct hfp_hf *hfp;
>> +
>> + if (fd < 0)
>> + return NULL;
>> +
>> + hfp = new0(struct hfp_hf, 1);
>> + if (!hfp)
>> + return NULL;
>> +
>> + hfp->fd = fd;
>> + hfp->close_on_unref = false;
>> +
>> + hfp->read_buf = ringbuf_new(4096);
>> + if (!hfp->read_buf) {
>> + free(hfp);
>> + return NULL;
>> + }
>> +
>> + hfp->write_buf = ringbuf_new(4096);
>> + if (!hfp->write_buf) {
>> + ringbuf_free(hfp->read_buf);
>> + free(hfp);
>> + return NULL;
>> + }
>> +
>> + hfp->io = io_new(fd);
>> + if (!hfp->io) {
>> + ringbuf_free(hfp->write_buf);
>> + ringbuf_free(hfp->read_buf);
>> + free(hfp);
>> + return NULL;
>> + }
>> +
>> + return hfp_hf_ref(hfp);
>> +}
>> +
>> +struct hfp_hf *hfp_hf_ref(struct hfp_hf *hfp)
>> +{
>> + if (!hfp)
>> + return NULL;
>> +
>> + __sync_fetch_and_add(&hfp->ref_count, 1);
>> +
>> + return hfp;
>> +}
>> +
>> +void hfp_hf_unref(struct hfp_hf *hfp)
>> +{
>> + if (!hfp)
>> + return;
>> +
>> + if (__sync_sub_and_fetch(&hfp->ref_count, 1))
>> + return;
>> +
>> + io_set_write_handler(hfp->io, NULL, NULL, NULL);
>> + io_set_read_handler(hfp->io, NULL, NULL, NULL);
>> + io_set_disconnect_handler(hfp->io, NULL, NULL, NULL);
>> +
>> + io_destroy(hfp->io);
>> + hfp->io = NULL;
>> +
>> + if (hfp->close_on_unref)
>> + close(hfp->fd);
>> +
>> + ringbuf_free(hfp->read_buf);
>> + hfp->read_buf = NULL;
>> +
>> + ringbuf_free(hfp->write_buf);
>> + hfp->write_buf = NULL;
>> +
>> + if (!hfp->in_disconnect) {
>> + free(hfp);
>> + return;
>> + }
>> +
>> + hfp->destroyed = true;
>> +}
>> diff --git a/src/shared/hfp.h b/src/shared/hfp.h
>> index 743db65..50d9c4b 100644
>> --- a/src/shared/hfp.h
>> +++ b/src/shared/hfp.h
>> @@ -76,9 +76,11 @@ typedef void (*hfp_destroy_func_t)(void *user_data);
>> typedef void (*hfp_debug_func_t)(const char *str, void *user_data);
>>
>> typedef void (*hfp_command_func_t)(const char *command, void *user_data);
>> +
>
> Unrelated.
True.
>
>> typedef void (*hfp_disconnect_func_t)(void *user_data);
>>
>> struct hfp_gw;
>> +struct hfp_hf;
>
> I'd prefer if we have all hfp_hf stuff in same section.
OK
>
>>
>> struct hfp_gw *hfp_gw_new(int fd);
>>
>> @@ -124,3 +126,7 @@ bool hfp_gw_result_get_string(struct hfp_gw_result *result, char *buf,
>> bool hfp_gw_result_get_unquoted_string(struct hfp_gw_result *result, char *buf,
>> uint8_t len);
>> bool hfp_gw_result_has_next(struct hfp_gw_result *result);
>> +
>> +struct hfp_hf *hfp_hf_new(int fd);
>> +struct hfp_hf *hfp_hf_ref(struct hfp_hf *hfp);
>> +void hfp_hf_unref(struct hfp_hf *hfp);
>>
>
> --
> Best regards,
> Szymon Janc
\Łukasz
^ permalink raw reply
* Re: [PATCH v4 05/11] shared/hfp: Add HFP HF parser
From: Lukasz Rymanowski @ 2014-10-23 15:00 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <2626729.yqIRdlFYHe@uw000953>
Hi Szymon,
On 22 October 2014 13:00, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi Łukasz,
>
> On Friday 10 of October 2014 01:50:05 Lukasz Rymanowski wrote:
>> This patch adds parser for AT responses and unsolicited results.
>> ---
>> src/shared/hfp.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 129 insertions(+)
>>
>> diff --git a/src/shared/hfp.c b/src/shared/hfp.c
>> index b1cf08e..5179092 100644
>> --- a/src/shared/hfp.c
>> +++ b/src/shared/hfp.c
>> @@ -868,6 +868,126 @@ static void destroy_event_handler(void *data)
>> free(handler);
>> }
>>
>> +static void hf_skip_whitespace(struct hfp_hf_result *result)
>> +{
>> + while (result->data[result->offset] == ' ')
>> + result->offset++;
>> +}
>> +
>> +static void hf_call_prefix_handler(struct hfp_hf *hfp, const char *data)
>> +{
>> + struct event_handler *handler;
>> + const char *separators = ";:\0";
>> + struct hfp_hf_result result_data;
>> + char lookup_prefix[18];
>> + uint8_t pref_len = 0;
>> + const char *prefix;
>> + int i;
>> +
>> + result_data.offset = 0;
>> + result_data.data = data;
>> +
>> + hf_skip_whitespace(&result_data);
>> +
>> + if (strlen(data + result_data.offset) < 2)
>> + return;
>> +
>> + prefix = data + result_data.offset;
>> +
>> + pref_len = strcspn(prefix, separators);
>> + if (pref_len > 17 || pref_len < 2)
>> + return;
>> +
>> + for (i = 0; i < pref_len; i++)
>> + lookup_prefix[i] = toupper(prefix[i]);
>> +
>> + lookup_prefix[pref_len] = '\0';
>> + result_data.offset += pref_len + 1;
>> +
>> + handler = queue_find(hfp->event_handlers, match_handler_event_prefix,
>> + lookup_prefix);
>> + if (!handler)
>> + return;
>> +
>> + handler->callback(&result_data, handler->user_data);
>> +}
>> +
>> +static char *find_cr_lf(char *str, size_t len)
>> +{
>> + char *ptr;
>> + int count;
>> + int offset;
>> +
>> + offset = 0;
>> +
>> + ptr = memchr(str, '\r', len);
>> + while (ptr) {
>> + /*
>> + * Check if there is more data after '\r'. If so check for
>> + * '\n'
>> + */
>> + count = ptr-str;
>
> Style: spaces around -
>
>> + if ((count < (int) (len - 1)) && *(ptr + 1) == '\n')
>> + return ptr;
>
> If you make count size_t then this cast is not needed.
>
>> +
>> + /* There is only '\r'? Let's try to find next one */
>> + offset += count + 1;
>> +
>> + if (offset >= (int)len)
>
> If you make offset size_t then this cast is not needed.
> Also such casting should have space '(int) foo'.
>
>> + return NULL;
>> +
>> + ptr = memchr(str + offset, '\r', len - offset);
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static void hf_process_input(struct hfp_hf *hfp)
>> +{
>> + char *str, *ptr;
>> + size_t len, count, offset;
>> +
>> + str = ringbuf_peek(hfp->read_buf, 0, &len);
>> + if (!str)
>> + return;
>> +
>> + offset = 0;
>> +
>> + ptr = find_cr_lf(str, len);
>> + while (ptr) {
>> + count = ptr - (str + offset);
>
> If you would adjust str pointer instead of using str + offset everywhere
> then this code would be a bit simpler to follow.
>
see below
> Also this would not handle wrapped string correctly. Check how this is handled
> in process_input().
Missed that. Will fix, also will fix that code as asprintf should not
be use. str is not a string in case buffer is wrapped.
Also will need to keep offset so I can concatenate str and str2 which
will come from the begging of ringbuf
>
>> + if (count == 0) {
>> + /* 2 is for <cr><lf> */
>> + offset += 2;
>> + } else {
>> + *ptr = '\0';
>> + hf_call_prefix_handler(hfp, str + offset);
>> + offset += count + 2;
>> + }
>> +
>> + if (offset >= len)
>> + break;
>> +
>> + ptr = find_cr_lf(str + offset, len - offset);
>> + }
>> +
>> + ringbuf_drain(hfp->read_buf, offset < len ? offset : len);
>> +}
>> +
>> +static bool hf_can_read_data(struct io *io, void *user_data)
>> +{
>> + struct hfp_hf *hfp = user_data;
>> + ssize_t bytes_read;
>> +
>> + bytes_read = ringbuf_read(hfp->read_buf, hfp->fd);
>> + if (bytes_read < 0)
>> + return false;
>> +
>> + hf_process_input(hfp);
>> +
>> + return true;
>> +}
>> +
>> struct hfp_hf *hfp_hf_new(int fd)
>> {
>> struct hfp_hf *hfp;
>> @@ -912,6 +1032,15 @@ struct hfp_hf *hfp_hf_new(int fd)
>> return NULL;
>> }
>>
>> + if (!io_set_read_handler(hfp->io, hf_can_read_data, hfp,
>> + read_watch_destroy)) {
>> + queue_destroy(hfp->event_handlers,
>> + destroy_event_handler);
>> + io_destroy(hfp->io);
>> + ringbuf_free(hfp->write_buf);
>> + ringbuf_free(hfp->read_buf);
>
> You are missing free(hfp); return NULL; here.
ah, some rebase issue. thanks
>
>> + }
>> +
>> return hfp_hf_ref(hfp);
>> }
>>
>>
>
> --
> Best regards,
> Szymon Janc
\Łukasz
^ permalink raw reply
* Re: [PATCH v4 04/11] shared/hfp: Add register/unregister event for HFP HF
From: Lukasz Rymanowski @ 2014-10-23 15:00 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <11593362.6Zs3oQ3sJP@uw000953>
Hi Szymon,
On 22 October 2014 13:00, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi Łukasz,
>
> On Friday 10 of October 2014 01:50:04 Lukasz Rymanowski wrote:
>> This patch adds API which allows to register/unregister for unsolicited
>> responses.
>> ---
>> src/shared/hfp.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> src/shared/hfp.h | 8 +++++
>> 2 files changed, 116 insertions(+)
>>
>> diff --git a/src/shared/hfp.c b/src/shared/hfp.c
>> index b7855ed..b1cf08e 100644
>> --- a/src/shared/hfp.c
>> +++ b/src/shared/hfp.c
>> @@ -70,6 +70,8 @@ struct hfp_hf {
>> struct ringbuf *read_buf;
>> struct ringbuf *write_buf;
>>
>> + struct queue *event_handlers;
>> +
>> hfp_debug_func_t debug_callback;
>> hfp_destroy_func_t debug_destroy;
>> void *debug_data;
>> @@ -94,6 +96,18 @@ struct hfp_gw_result {
>> unsigned int offset;
>> };
>>
>> +struct hfp_hf_result {
>> + const char *data;
>> + unsigned int offset;
>> +};
>> +
>> +struct event_handler {
>> + char *prefix;
>> + void *user_data;
>> + hfp_destroy_func_t destroy;
>> + hfp_hf_result_func_t callback;
>> +};
>> +
>> static void destroy_cmd_handler(void *data)
>> {
>> struct cmd_handler *handler = data;
>> @@ -828,6 +842,32 @@ bool hfp_gw_disconnect(struct hfp_gw *hfp)
>> return io_shutdown(hfp->io);
>> }
>>
>> +static bool match_handler_event_prefix(const void *a, const void *b)
>> +{
>> + const struct event_handler *handler = a;
>> + const char *prefix = b;
>> +
>> + if (strlen(handler->prefix) != strlen(prefix))
>> + return false;
>> +
>> + if (memcmp(handler->prefix, prefix, strlen(prefix)))
>> + return false;
>
> Why not just use strcmp() here?
> I'm aware that this was based on gw code so you may also fix it there :)
Copy paste issue. But that's correct. Will send also patch for base code.
>
>> +
>> + return true;
>> +}
>> +
>> +static void destroy_event_handler(void *data)
>> +{
>> + struct event_handler *handler = data;
>> +
>> + if (handler->destroy)
>> + handler->destroy(handler->user_data);
>> +
>> + free(handler->prefix);
>> +
>> + free(handler);
>> +}
>> +
>> struct hfp_hf *hfp_hf_new(int fd)
>> {
>> struct hfp_hf *hfp;
>> @@ -863,6 +903,15 @@ struct hfp_hf *hfp_hf_new(int fd)
>> return NULL;
>> }
>>
>> + hfp->event_handlers = queue_new();
>> + if (!hfp->event_handlers) {
>> + io_destroy(hfp->io);
>> + ringbuf_free(hfp->write_buf);
>> + ringbuf_free(hfp->read_buf);
>> + free(hfp);
>> + return NULL;
>> + }
>> +
>> return hfp_hf_ref(hfp);
>> }
>>
>> @@ -902,6 +951,9 @@ void hfp_hf_unref(struct hfp_hf *hfp)
>> ringbuf_free(hfp->write_buf);
>> hfp->write_buf = NULL;
>>
>> + queue_destroy(hfp->event_handlers, destroy_event_handler);
>> + hfp->event_handlers = NULL;
>> +
>> if (!hfp->in_disconnect) {
>> free(hfp);
>> return;
>> @@ -961,6 +1013,62 @@ bool hfp_hf_set_close_on_unref(struct hfp_hf *hfp, bool do_close)
>> return true;
>> }
>>
>> +bool hfp_hf_register(struct hfp_hf *hfp, hfp_hf_result_func_t callback,
>> + const char *prefix,
>> + void *user_data,
>> + hfp_destroy_func_t destroy)
>> +{
>> + struct event_handler *handler;
>> +
>> + if (!callback)
>> + return false;
>> +
>> + handler = new0(struct event_handler, 1);
>> + if (!handler)
>> + return false;
>> +
>> + handler->callback = callback;
>> + handler->user_data = user_data;
>> +
>> + handler->prefix = strdup(prefix);
>> + if (!handler->prefix) {
>> + free(handler);
>> + return false;
>> + }
>> +
>> + if (queue_find(hfp->event_handlers, match_handler_event_prefix,
>> + handler->prefix)) {
>> + destroy_event_handler(handler);
>> + return false;
>> + }
>> +
>> + handler->destroy = destroy;
>> +
>> + return queue_push_tail(hfp->event_handlers, handler);
>> +}
>> +
>> +bool hfp_hf_unregister(struct hfp_hf *hfp, const char *prefix)
>> +{
>> + struct cmd_handler *handler;
>> + char *lookup_prefix;
>> +
>> + lookup_prefix = strdup(prefix);
>> + if (!lookup_prefix)
>> + return false;
>
> This strdup seems superfluous. If this is only due to to queue api being
> non-const then I'd just cast it to (void *).
:) ditto.
>
>> +
>> + handler = queue_remove_if(hfp->event_handlers,
>> + match_handler_event_prefix,
>> + lookup_prefix);
>> + free(lookup_prefix);
>> +
>> + if (!handler)
>> + return false;
>> +
>> + destroy_event_handler(handler);
>> +
>> + return true;
>> +}
>> +
>> static void hf_disconnect_watch_destroy(void *user_data)
>> {
>> struct hfp_hf *hfp = user_data;
>> diff --git a/src/shared/hfp.h b/src/shared/hfp.h
>> index a9a169b..85037b1 100644
>> --- a/src/shared/hfp.h
>> +++ b/src/shared/hfp.h
>> @@ -68,10 +68,14 @@ enum hfp_gw_cmd_type {
>> };
>>
>> struct hfp_gw_result;
>> +struct hfp_hf_result;
>
> As before, I'd keep hf stuff in single section.
This and the one below will finally goes out as it will be merget into
hfp_context. I think there is no sense to change that here now.
>
>>
>> typedef void (*hfp_result_func_t)(struct hfp_gw_result *result,
>> enum hfp_gw_cmd_type type, void *user_data);
>>
>> +typedef void (*hfp_hf_result_func_t)(struct hfp_hf_result *result,
>> + void *user_data);
>> +
>
> Same here.
ditto.
>
>> typedef void (*hfp_destroy_func_t)(void *user_data);
>> typedef void (*hfp_debug_func_t)(const char *str, void *user_data);
>>
>> @@ -138,3 +142,7 @@ bool hfp_hf_set_disconnect_handler(struct hfp_hf *hfp,
>> void *user_data,
>> hfp_destroy_func_t destroy);
>> bool hfp_hf_disconnect(struct hfp_hf *hfp);
>> +bool hfp_hf_register(struct hfp_hf *hfp, hfp_hf_result_func_t callback,
>> + const char *prefix, void *user_data,
>> + hfp_destroy_func_t destroy);
>> +bool hfp_hf_unregister(struct hfp_hf *hfp, const char *prefix);
>>
>
> --
> Best regards,
> Szymon Janc
^ permalink raw reply
* [PATCH v5 bluetooth-next 4/4] ieee802154: 6lowpan: rename process_data and lowpan_process_data
From: Martin Townsend @ 2014-10-23 14:40 UTC (permalink / raw)
To: linux-bluetooth, linux-wpan
Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend
In-Reply-To: <1414075256-9448-1-git-send-email-martin.townsend@xsilon.com>
From: Martin Townsend <mtownsend1973@gmail.com>
As we have decouple decompression from data delivery we can now rename all
occurences of process_data in receive path.
Signed-off-by: Martin Townsend <mtownsend1973@gmail.com>
---
include/net/6lowpan.h | 10 ++++++----
net/6lowpan/iphc.c | 12 +++++++-----
net/bluetooth/6lowpan.c | 15 ++++++++-------
net/ieee802154/6lowpan_rtnl.c | 15 ++++++++-------
4 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index abfa359..dc03d77 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -372,10 +372,12 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
return skb->len + uncomp_header - ret;
}
-int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
- const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
- const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
- u8 iphc0, u8 iphc1);
+int
+lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev,
+ const u8 *saddr, const u8 saddr_type,
+ const u8 saddr_len, const u8 *daddr,
+ const u8 daddr_type, const u8 daddr_len,
+ u8 iphc0, u8 iphc1);
int lowpan_header_compress(struct sk_buff *skb, struct net_device *dev,
unsigned short type, const void *_daddr,
const void *_saddr, unsigned int len);
diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 45714fe..73a7065 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -301,10 +301,12 @@ err:
/* TTL uncompression values */
static const u8 lowpan_ttl_values[] = { 0, 1, 64, 255 };
-int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
- const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
- const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
- u8 iphc0, u8 iphc1)
+int
+lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev,
+ const u8 *saddr, const u8 saddr_type,
+ const u8 saddr_len, const u8 *daddr,
+ const u8 daddr_type, const u8 daddr_len,
+ u8 iphc0, u8 iphc1)
{
struct ipv6hdr hdr = {};
u8 tmp, num_context = 0;
@@ -480,7 +482,7 @@ drop:
kfree_skb(skb);
return -EINVAL;
}
-EXPORT_SYMBOL_GPL(lowpan_process_data);
+EXPORT_SYMBOL_GPL(lowpan_header_decompress);
static u8 lowpan_compress_addr_64(u8 **hc_ptr, u8 shift,
const struct in6_addr *ipaddr,
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 40e2cec..aa6ebbf 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -257,8 +257,8 @@ static int give_skb_to_upper(struct sk_buff *skb, struct net_device *dev)
return netif_rx(skb_cp);
}
-static int process_data(struct sk_buff *skb, struct net_device *netdev,
- struct l2cap_chan *chan)
+static int iphc_decompress(struct sk_buff *skb, struct net_device *netdev,
+ struct l2cap_chan *chan)
{
const u8 *saddr, *daddr;
u8 iphc0, iphc1;
@@ -287,10 +287,11 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev,
if (lowpan_fetch_skb_u8(skb, &iphc1))
goto drop;
- return lowpan_process_data(skb, netdev,
- saddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
- daddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
- iphc0, iphc1);
+ return lowpan_header_decompress(skb, netdev,
+ saddr, IEEE802154_ADDR_LONG,
+ EUI64_ADDR_LEN, daddr,
+ IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
+ iphc0, iphc1);
drop:
kfree_skb(skb);
@@ -346,7 +347,7 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
if (!local_skb)
goto drop;
- ret = process_data(local_skb, dev, chan);
+ ret = iphc_decompress(local_skb, dev, chan);
if (ret < 0)
goto drop;
diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 577597d..d413a60 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -166,7 +166,8 @@ static int lowpan_give_skb_to_devices(struct sk_buff *skb,
return stat;
}
-static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
+static int
+iphc_decompress(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
{
u8 iphc0, iphc1;
struct ieee802154_addr_sa sa, da;
@@ -196,9 +197,9 @@ static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
else
dap = &da.hwaddr;
- return lowpan_process_data(skb, skb->dev, sap, sa.addr_type,
- IEEE802154_ADDR_LEN, dap, da.addr_type,
- IEEE802154_ADDR_LEN, iphc0, iphc1);
+ return lowpan_header_decompress(skb, skb->dev, sap, sa.addr_type,
+ IEEE802154_ADDR_LEN, dap, da.addr_type,
+ IEEE802154_ADDR_LEN, iphc0, iphc1);
drop:
kfree_skb(skb);
@@ -541,7 +542,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
} else {
switch (skb->data[0] & 0xe0) {
case LOWPAN_DISPATCH_IPHC: /* ipv6 datagram */
- ret = process_data(skb, &hdr);
+ ret = iphc_decompress(skb, &hdr);
if (ret < 0)
goto drop;
@@ -549,7 +550,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
case LOWPAN_DISPATCH_FRAG1: /* first fragment header */
ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
if (ret == 1) {
- ret = process_data(skb, &hdr);
+ ret = iphc_decompress(skb, &hdr);
if (ret < 0)
goto drop;
@@ -562,7 +563,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
case LOWPAN_DISPATCH_FRAGN: /* next fragments headers */
ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAGN);
if (ret == 1) {
- ret = process_data(skb, &hdr);
+ ret = iphc_decompress(skb, &hdr);
if (ret < 0)
goto drop;
--
1.9.1
^ permalink raw reply related
* [PATCH v5 bluetooth-next 3/4] bluetooth:6lowpan: use consume_skb when packet processed successfully
From: Martin Townsend @ 2014-10-23 14:40 UTC (permalink / raw)
To: linux-bluetooth, linux-wpan
Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend
In-Reply-To: <1414075256-9448-1-git-send-email-martin.townsend@xsilon.com>
From: Martin Townsend <mtownsend1973@gmail.com>
Signed-off-by: Martin Townsend <mtownsend1973@gmail.com>
---
net/bluetooth/6lowpan.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 94bbb66..40e2cec 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -337,8 +337,8 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
dev->stats.rx_bytes += skb->len;
dev->stats.rx_packets++;
- kfree_skb(local_skb);
- kfree_skb(skb);
+ consume_skb(local_skb);
+ consume_skb(skb);
} else {
switch (skb->data[0] & 0xe0) {
case LOWPAN_DISPATCH_IPHC: /* ipv6 datagram */
@@ -363,7 +363,8 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
dev->stats.rx_bytes += skb->len;
dev->stats.rx_packets++;
- kfree_skb(skb);
+ consume_skb(local_skb);
+ consume_skb(skb);
break;
default:
break;
--
1.9.1
^ permalink raw reply related
* [PATCH v5 bluetooth-next 2/4] 6lowpan: fix process_data return values
From: Martin Townsend @ 2014-10-23 14:40 UTC (permalink / raw)
To: linux-bluetooth, linux-wpan
Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend,
Martin Townsend
In-Reply-To: <1414075256-9448-1-git-send-email-martin.townsend@xsilon.com>
As process_data now returns just error codes fix up the calls to this
function to only drop the skb if an error code is returned.
Signed-off-by: Martin Townsend <mtownsend1973@gmail.com>
---
net/bluetooth/6lowpan.c | 2 +-
net/ieee802154/6lowpan_rtnl.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 45d9a9f..94bbb66 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -347,7 +347,7 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
goto drop;
ret = process_data(local_skb, dev, chan);
- if (ret != NET_RX_SUCCESS)
+ if (ret < 0)
goto drop;
local_skb->protocol = htons(ETH_P_IPV6);
diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 09feacf..577597d 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -542,7 +542,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
switch (skb->data[0] & 0xe0) {
case LOWPAN_DISPATCH_IPHC: /* ipv6 datagram */
ret = process_data(skb, &hdr);
- if (ret == NET_RX_DROP)
+ if (ret < 0)
goto drop;
return lowpan_give_skb_to_devices(skb, NULL);
@@ -550,7 +550,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
if (ret == 1) {
ret = process_data(skb, &hdr);
- if (ret == NET_RX_DROP)
+ if (ret < 0)
goto drop;
return lowpan_give_skb_to_devices(skb, NULL);
@@ -563,7 +563,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAGN);
if (ret == 1) {
ret = process_data(skb, &hdr);
- if (ret == NET_RX_DROP)
+ if (ret < 0)
goto drop;
return lowpan_give_skb_to_devices(skb, NULL);
--
1.9.1
^ permalink raw reply related
* [PATCH v5 bluetooth-next 1/4] 6lowpan: remove skb_deliver from IPHC
From: Martin Townsend @ 2014-10-23 14:40 UTC (permalink / raw)
To: linux-bluetooth, linux-wpan
Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend,
Martin Townsend
In-Reply-To: <1414075256-9448-1-git-send-email-martin.townsend@xsilon.com>
Separating skb delivery from decompression ensures that we can support further
decompression schemes and removes the mixed return value of error codes with
NET_RX_FOO.
Signed-off-by: Martin Townsend <mtownsend1973@gmail.com>
---
include/net/6lowpan.h | 4 +---
net/6lowpan/iphc.c | 32 ++++++--------------------------
net/bluetooth/6lowpan.c | 14 ++++++++++++--
net/ieee802154/6lowpan_rtnl.c | 41 ++++++++++++++++++++++++++---------------
4 files changed, 45 insertions(+), 46 deletions(-)
diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index d184df1..abfa359 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -372,12 +372,10 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
return skb->len + uncomp_header - ret;
}
-typedef int (*skb_delivery_cb)(struct sk_buff *skb, struct net_device *dev);
-
int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
- u8 iphc0, u8 iphc1, skb_delivery_cb skb_deliver);
+ u8 iphc0, u8 iphc1);
int lowpan_header_compress(struct sk_buff *skb, struct net_device *dev,
unsigned short type, const void *_daddr,
const void *_saddr, unsigned int len);
diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 747b3cc..45714fe 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -171,29 +171,6 @@ static int uncompress_context_based_src_addr(struct sk_buff *skb,
return 0;
}
-static int skb_deliver(struct sk_buff *skb, struct ipv6hdr *hdr,
- struct net_device *dev, skb_delivery_cb deliver_skb)
-{
- int stat;
-
- skb_push(skb, sizeof(struct ipv6hdr));
- skb_reset_network_header(skb);
- skb_copy_to_linear_data(skb, hdr, sizeof(struct ipv6hdr));
-
- skb->protocol = htons(ETH_P_IPV6);
- skb->pkt_type = PACKET_HOST;
- skb->dev = dev;
-
- raw_dump_table(__func__, "raw skb data dump before receiving",
- skb->data, skb->len);
-
- stat = deliver_skb(skb, dev);
-
- consume_skb(skb);
-
- return stat;
-}
-
/* Uncompress function for multicast destination address,
* when M bit is set.
*/
@@ -327,7 +304,7 @@ static const u8 lowpan_ttl_values[] = { 0, 1, 64, 255 };
int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
- u8 iphc0, u8 iphc1, skb_delivery_cb deliver_skb)
+ u8 iphc0, u8 iphc1)
{
struct ipv6hdr hdr = {};
u8 tmp, num_context = 0;
@@ -492,10 +469,13 @@ int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
hdr.version, ntohs(hdr.payload_len), hdr.nexthdr,
hdr.hop_limit, &hdr.daddr);
- raw_dump_table(__func__, "raw header dump", (u8 *)&hdr, sizeof(hdr));
+ skb_push(skb, sizeof(hdr));
+ skb_reset_network_header(skb);
+ skb_copy_to_linear_data(skb, &hdr, sizeof(hdr));
- return skb_deliver(skb, &hdr, dev, deliver_skb);
+ raw_dump_table(__func__, "raw header dump", (u8 *)&hdr, sizeof(hdr));
+ return 0;
drop:
kfree_skb(skb);
return -EINVAL;
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 6c5c2ef..45d9a9f 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -252,7 +252,7 @@ static int give_skb_to_upper(struct sk_buff *skb, struct net_device *dev)
skb_cp = skb_copy(skb, GFP_ATOMIC);
if (!skb_cp)
- return -ENOMEM;
+ return NET_RX_DROP;
return netif_rx(skb_cp);
}
@@ -290,7 +290,7 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev,
return lowpan_process_data(skb, netdev,
saddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
daddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
- iphc0, iphc1, give_skb_to_upper);
+ iphc0, iphc1);
drop:
kfree_skb(skb);
@@ -350,6 +350,16 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
if (ret != NET_RX_SUCCESS)
goto drop;
+ local_skb->protocol = htons(ETH_P_IPV6);
+ local_skb->pkt_type = PACKET_HOST;
+ local_skb->dev = dev;
+
+ if (give_skb_to_upper(local_skb, dev)
+ != NET_RX_SUCCESS) {
+ kfree_skb(local_skb);
+ goto drop;
+ }
+
dev->stats.rx_bytes += skb->len;
dev->stats.rx_packets++;
diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 0c1a49b..09feacf 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -141,20 +141,28 @@ static int lowpan_give_skb_to_devices(struct sk_buff *skb,
struct sk_buff *skb_cp;
int stat = NET_RX_SUCCESS;
+ skb->protocol = htons(ETH_P_IPV6);
+ skb->pkt_type = PACKET_HOST;
+
rcu_read_lock();
list_for_each_entry_rcu(entry, &lowpan_devices, list)
if (lowpan_dev_info(entry->ldev)->real_dev == skb->dev) {
skb_cp = skb_copy(skb, GFP_ATOMIC);
if (!skb_cp) {
- stat = -ENOMEM;
- break;
+ kfree_skb(skb);
+ rcu_read_unlock();
+ return NET_RX_DROP;
}
skb_cp->dev = entry->ldev;
stat = netif_rx(skb_cp);
+ if (stat == NET_RX_DROP)
+ break;
}
rcu_read_unlock();
+ consume_skb(skb);
+
return stat;
}
@@ -190,8 +198,7 @@ static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
return lowpan_process_data(skb, skb->dev, sap, sa.addr_type,
IEEE802154_ADDR_LEN, dap, da.addr_type,
- IEEE802154_ADDR_LEN, iphc0, iphc1,
- lowpan_give_skb_to_devices);
+ IEEE802154_ADDR_LEN, iphc0, iphc1);
drop:
kfree_skb(skb);
@@ -528,44 +535,48 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
/* check that it's our buffer */
if (skb->data[0] == LOWPAN_DISPATCH_IPV6) {
- skb->protocol = htons(ETH_P_IPV6);
- skb->pkt_type = PACKET_HOST;
-
/* Pull off the 1-byte of 6lowpan header. */
skb_pull(skb, 1);
-
- ret = lowpan_give_skb_to_devices(skb, NULL);
- if (ret == NET_RX_DROP)
- goto drop;
+ return lowpan_give_skb_to_devices(skb, NULL);
} else {
switch (skb->data[0] & 0xe0) {
case LOWPAN_DISPATCH_IPHC: /* ipv6 datagram */
ret = process_data(skb, &hdr);
if (ret == NET_RX_DROP)
goto drop;
- break;
+
+ return lowpan_give_skb_to_devices(skb, NULL);
case LOWPAN_DISPATCH_FRAG1: /* first fragment header */
ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
if (ret == 1) {
ret = process_data(skb, &hdr);
if (ret == NET_RX_DROP)
goto drop;
+
+ return lowpan_give_skb_to_devices(skb, NULL);
+ } else if (ret == -1) {
+ return NET_RX_DROP;
+ } else {
+ return NET_RX_SUCCESS;
}
- break;
case LOWPAN_DISPATCH_FRAGN: /* next fragments headers */
ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAGN);
if (ret == 1) {
ret = process_data(skb, &hdr);
if (ret == NET_RX_DROP)
goto drop;
+
+ return lowpan_give_skb_to_devices(skb, NULL);
+ } else if (ret == -1) {
+ return NET_RX_DROP;
+ } else {
+ return NET_RX_SUCCESS;
}
- break;
default:
break;
}
}
- return NET_RX_SUCCESS;
drop_skb:
kfree_skb(skb);
drop:
--
1.9.1
^ permalink raw reply related
* [PATCH v5 bluetooth-next 0/4] 6lowpan: Move skb delivery out of IPHC
From: Martin Townsend @ 2014-10-23 14:40 UTC (permalink / raw)
To: linux-bluetooth, linux-wpan
Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend
This series moves skb delivery out of IPHC and into the receive routines of
both bluetooth and 802.15.4. The reason is that we need to support more
(de)compression schemes in the future. It also means that calling
lowpan_process_data now only returns error codes or 0 for success so
this has been cleaned up. The final patch then renames occurences of
lowpan_process_data and process_data to something more meaningful.
v1 -> v2
* Handle freeing of skb in lowpan_give_skb_to_devices for 802.15.14
* Added another skb_consume in bluetooth code for uncompressed IPv6 headers
* Added missing skb_consume for local_skb for bluetooth in compreesed IPv6 header
path
* Combine patch 4 into 1.
v2 -> v3
* Ensure error codes aren't returned in the bluetooth skb delivery function.
* In lowpan_give_skb_to_devices return NET_RX_DROP when the first call to
netif_rx fails.
v3 -> v4
* Fixed incorrect frag return handling due to refactoring skb_deliver.
v4 -> v5
* Removed unnecessary break statements in lowpan_rcv
Martin Townsend (4):
6lowpan: remove skb_deliver from IPHC
6lowpan: fix process_data return values
bluetooth:6lowpan: use consume_skb when packet processed successfully
ieee802154: 6lowpan: rename process_data and lowpan_process_data
include/net/6lowpan.h | 12 ++++-----
net/6lowpan/iphc.c | 42 +++++++++---------------------
net/bluetooth/6lowpan.c | 36 +++++++++++++++++---------
net/ieee802154/6lowpan_rtnl.c | 60 ++++++++++++++++++++++++++-----------------
4 files changed, 78 insertions(+), 72 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH 2/2] sbc: use an uint16 to store frame length in internal frame structure
From: Aurélien Zanelli @ 2014-10-23 14:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Aurélien Zanelli
In-Reply-To: <1414074741-20286-1-git-send-email-aurelien.zanelli@parrot.com>
Otherwise it could overflow in some cases.
For instance in DUAL_CHANNEL mode, with subbands set to SBC_SB_8, blocks
set to SBC_BLK_16 and bitpool set to 64 results in a frame length of 268.
---
sbc/sbc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sbc/sbc.c b/sbc/sbc.c
index e830388..606f11c 100644
--- a/sbc/sbc.c
+++ b/sbc/sbc.c
@@ -119,7 +119,7 @@ struct sbc_frame {
uint8_t subbands;
uint8_t bitpool;
uint16_t codesize;
- uint8_t length;
+ uint16_t length;
/* bit number x set means joint stereo has been used in subband x */
uint8_t joint;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/2] sbc: fix frame length calculation for DUAL_CHANNEL mode
From: Aurélien Zanelli @ 2014-10-23 14:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Aurélien Zanelli
In-Reply-To: <1414074741-20286-1-git-send-email-aurelien.zanelli@parrot.com>
According to A2DP specification, section 12.9, for DUAL_CHANNEL mode, we
shall use the same formula as for MONO mode.
---
sbc/sbc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sbc/sbc.c b/sbc/sbc.c
index 534027e..e830388 100644
--- a/sbc/sbc.c
+++ b/sbc/sbc.c
@@ -1402,7 +1402,7 @@ SBC_EXPORT size_t sbc_get_frame_length(sbc_t *sbc)
ret = 4 + (4 * subbands * channels) / 8;
/* This term is not always evenly divide so we round it up */
- if (channels == 1)
+ if (channels == 1 || sbc->mode == SBC_MODE_DUAL_CHANNEL)
ret += ((blocks * channels * bitpool) + 7) / 8;
else
ret += (((joint ? subbands : 0) + blocks * bitpool) + 7) / 8;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/2] sbc: fix frame length in DUAL_CHANNEL mode
From: Aurélien Zanelli @ 2014-10-23 14:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Aurélien Zanelli
Hi everyone,
I was playing with sbc encoder in GStreamer, which use bluez sbc library to
encode and I found two issues related to DUAL_CHANNEL mode in it.
First the frame length calculation for DUAL_CHANNEL mode was wrong. I found
the A2DP specification and it seems that for DUAL_CHANNEL mode, we should use
the same formula as MONO mode to calculate the frame length. The first patch
intend to fix this.
The second patch try to fix an overflow which can occur when the frame length
is greater than 255.
Aurélien Zanelli (2):
sbc: fix frame length calculation for DUAL_CHANNEL mode
sbc: use an uint16 to store frame length in internal frame structure
sbc/sbc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
1.7.10.4
^ permalink raw reply
* Re: [PATCH v3 bluetooth-next 1/4] 6lowpan: remove skb_deliver from IPHC
From: Alexander Aring @ 2014-10-23 14:28 UTC (permalink / raw)
To: Martin Townsend
Cc: linux-bluetooth, linux-wpan, marcel, jukka.rissanen,
Martin Townsend
In-Reply-To: <1414069948-29731-2-git-send-email-martin.townsend@xsilon.com>
Hi Martin,
On Thu, Oct 23, 2014 at 02:12:25PM +0100, Martin Townsend wrote:
...
> } else {
> switch (skb->data[0] & 0xe0) {
> case LOWPAN_DISPATCH_IPHC: /* ipv6 datagram */
> ret = process_data(skb, &hdr);
> if (ret == NET_RX_DROP)
> goto drop;
> - break;
> +
> + return lowpan_give_skb_to_devices(skb, NULL);
> case LOWPAN_DISPATCH_FRAG1: /* first fragment header */
> ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
> if (ret == 1) {
> ret = process_data(skb, &hdr);
> if (ret == NET_RX_DROP)
> goto drop;
> +
> + return lowpan_give_skb_to_devices(skb, NULL);
> + } else if (ret == -1) {
> + return NET_RX_DROP;
> + } else {
> + return NET_RX_SUCCESS;
> }
> break;
break isn't necessary here.
> case LOWPAN_DISPATCH_FRAGN: /* next fragments headers */
> @@ -558,6 +566,12 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
> ret = process_data(skb, &hdr);
> if (ret == NET_RX_DROP)
> goto drop;
> +
> + return lowpan_give_skb_to_devices(skb, NULL);
> + } else if (ret == -1) {
> + return NET_RX_DROP;
> + } else {
> + return NET_RX_SUCCESS;
> }
> break;
same here.
You tagged this patch series as v3 but should be v4. Next series should
be v5.
- Alex
^ permalink raw reply
* Re: Attribute security permissions and CCC descriptors in shared/gatt-server.
From: Vinícius Costa Gomes @ 2014-10-23 13:53 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Arman Uguray, BlueZ development, Marcin Kraglak, Marcel Holtmann
In-Reply-To: <CABBYNZLE_xxoaPXZ=9LY2AwT-Pa_mqEYRD8v4_TgMTdo77j-Qw@mail.gmail.com>
Hi Luiz,
On Thu, Oct 23, 2014 at 5:51 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
[...]
>>
>>
>> *** 2. Is shared/gatt-server responsible for keeping track of Client
>> Characteristic Configuration descriptor states for each bonded client?
>> ***
>>
>> Currently, the descriptor read/write operations need to be handled by
>> the upper layer via the read/write callbacks anyway, since the
>> side-effects of a writing to a particular CCC descriptor need to be
>> implemented by the related profile. In the bonding case, the
>> read/write callbacks could determine what value to return and how to
>> cache the value on a per-device basis without shared/gatt-db having a
>> notion of "per-client CCC descriptors".
>
> Well making gatt_db have the notion of CCC would make things a little
> bit simpler for storing and reloading but it would probably force us
> to rethink how the db access the values, perhaps the db could cache
> CCC values per client so it could detect changes and send
> notifications automatically whenever the profiles tell it values has
> changed or when a write succeeds.
>
>> Does this make sense? If so, is there a point of keeping the "bdaddr_t
>> *bdaddr" arguments of gatt_db_read_t, gatt_db_write_t, gatt_db_read,
>> and gatt_db_write? Note that I'm currently passing NULL to all of
>> these calls for this parameter in shared/gatt-server, since bt_att
>> doesn't have a notion of a BD_ADDR.
>
> I guess this was inspired by Android, I would agree that if you
> register a characteristic and set the permission properly then it does
> not matter who is attempting to read/write as long as they are fulfill
> the requirements it should be okay but perhaps Android have pushed the
> notion of CCC up in the stack which force it to expose the remote
> address. Anyway for unit test I will have to address it since that
> will be using a socket pair, perhaps passing NULL is fine if we have
> CCC caching but in case of Android I don't think we can do much about
> it.
>
Don't know if I am following correctly here. But I guess that even
that, we would
need to have a way for the profile to identify who is writing in the CCC.
For example, in a (imaginary) Temperature profile that the client may
choose the unit (degrees Celsius or Fahrenheit) that is used for
indications/notifications. Each client would receive a different
notification value depending on what's written on the CCC.
Another solution would be for the "view" of the database that the profile has
depends on the device "operating" (not a very good word, because a
operation may be something like receiving a notification) on it. Just thinking
out loud.
>
> --
> Luiz Augusto von Dentz
> --
> 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
Cheers,
--
Vinicius
^ permalink raw reply
* Re: [bluetooth] btusb issues
From: Patrick Shirkey @ 2014-10-23 13:46 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <20141023122933.GA29656@aemeltch-mobl1.fi.intel.com>
On Thu, October 23, 2014 11:29 pm, Andrei Emeltchenko wrote:
> Hi Patrick,
>
> On Thu, Oct 23, 2014 at 09:59:23PM +1100, Patrick Shirkey wrote:
>>
>> On Thu, October 23, 2014 7:59 pm, Patrick Shirkey wrote:
>> > Hi,
>> >
>> > I have now got backports to run with bluez instead of bluedroid and
>> the
>> > btusb driver instead of rtk_btusb.
>> >
>> > However the android bluetooth system does not want to run for me.
>
> So what is not working? Do you have logcat with "BlueZ" and "bluetothd"
> logs?
>
It is working better now and I can see the BLE device in the Settings UI.
For reference I had to add the following insmod commands to my init file
to get all the backports drivers to load.
# Bluez bluetooth and backports modules
insmod /system/vendor/modules/compat.ko
insmod /system/vendor/modules/bluetooth.ko
insmod /system/vendor/modules/btusb.ko
insmod /system/vendor/modules/rfcomm.ko
insmod /system/vendor/modules/bnep.ko
And I had to double check that Crypto configs were set (which is
referenced in the aosp-bluez docs on the website):
CONFIG_CRYPTO_CMAC=y
CONFIG_CRYPTO_USER_API=y
CONFIG_CRYPTO_USER_API_HASH=y
CONFIG_CRYPTO_USER_API_SKCIPHER=y
> Best regards
> Andrei Emeltchenko
>
>> >
>>
>> For reference purposes I have got bluez running on an 8723au chipset
>> with
>> the btusb driver from the "new" branch of the btusb git repo.
>>
>> Here's an overview of the steps required:
>>
>> - Patch the 3.4 kernel with the patch files from the aosp-bluez "misc"
>> repo
>> - Completely replace bluedroid in the android tree with bluez including
>> the modifed bionic tree from the aosp-bluez repo
>> - Cross compile the backports drivers replacing btusb.c with the version
>> from the "new" branch in the git repo.
>> - Copy the modules from the backports tree to the linux tree before
>> building the firmware image. (overwrite the .ko files generated by the
>> kernel build)
>> - Include the firmware binary images from the git repo "new" branch in
>> the
>> android build so they are copied to the /system/etc/firmware/ folder in
>> the firmware image
>> - Modify the init.rc script to include the init.bluetooth.rc file
>> - Modify the <platform>.mk file (in this case
>> /device/softwinner/wing-k70/wing-k70.mk) to load the following modules
>> on
>> boot:
>> /system/vendor/modules/compat.ko
>> /system/vendor/modules/bluetooth.ko
>> /system/vendor/modules/btusb.ko
>>
>> enable debug logging with :
>>
>> setprop persist.sys.bluetooth.debug 1
>>
>>
>>
>> # btmgmt info
>> hci0: addr CC:D2:9C:73:CB:45 version 6 manufacturer 93 class 0x000000
>> supported settings: powered connectable fast-connectable discoverable
>> bondable link-security ssp br/edr hs le advertising debug-keys privacy
>> current settings: bondable ssp br/edr le
>> name BlueZ for Android
>> short name
>>
>>
>> # haltest
>> audio_hw_device_open returned -98
>> hw_get_module_by_class returned -2
>> thread_evt_cb: evt=ASSOCIATE_JVM
>> if_bluetooth->init: BT_STATUS_SUCCESS
>> get_profile_interface(handsfree) : 0xb6e58134
>> get_profile_interface(a2dp) : 0xb6e580ec
>> get_profile_interface(avrcp) : 0xb6e58100
>> get_profile_interface(health) : 0xb6e583d0
>> get_profile_interface(hidhost) : 0xb6e580a0
>> get_profile_interface(pan) : 0xb6e580d0
>> get_profile_interface(gatt) : 0xb6e581ac
>> get_profile_interface(socket) : 0xb6e58094
>> if_bluetooth->init: BT_STATUS_DONE
>> if_av->init: BT_STATUS_SUCCESS
>> if_rc->init: BT_STATUS_SUCCESS
>> if_gatt->init: BT_STATUS_FAIL
>> adapter_properties_cb: status=BT_STATUS_SUCCESS num_properties=1
>> prop: type=BT_PROPERTY_UUIDS len=80 val={0000110e, 0000110c, 0000110a,
>> 0000113b, 00001200}
>> adapter_properties_cb: status=BT_STATUS_SUCCESS num_properties=1
>> prop: type=BT_PROPERTY_UUIDS len=80 val={0000110e, 0000110c, 0000110a,
>> 0000113b, 00001200}
>> adapter_properties_cb: status=BT_STATUS_SUCCESS num_properties=1
>> prop: type=BT_PROPERTY_UUIDS len=80 val={0000110e, 0000110c, 0000110a,
>> 0000113b, 00001200}
>> if_hf->init: BT_STATUS_FAIL
>> if_hh->init: BT_STATUS_SUCCESS
>> if_pan->init: BT_STATUS_FAIL
>> adapter_properties_cb: status=BT_STATUS_SUCCESS num_properties=1
>> prop: type=BT_PROPERTY_UUIDS len=80 val={0000110e, 0000110c, 0000110a,
>> 0000113b, 00001200}
>> adapter_properties_cb: status=BT_STATUS_SUCCESS num_properties=1
>> prop: type=BT_PROPERTY_UUIDS len=80 val={0000110e, 0000110c, 0000110a,
>> 0000113b, 00001200}
>> if_hl->init: BT_STATUS_SUCCESS
>> >
>>
>>
>> # btmgmt le on
>> hci0 Set Low Energy complete, settings: bondable ssp br/edr le
>> # btmgmt find
>> Unable to start discovery. status 0x0f (Not Powered)
>> # btmgmt power on
>> hci0 Set Powered complete, settings: powered bondable ssp br/edr le
>> # btmgmt find
>> Discovery started
>> hci0 dev_found: BC:6A:29:AB:2C:AA type LE Public rssi -44 flags 0x0000
>> AD
>> flags 0x06 eir_len 25
>>
>>
>>
>>
>>
>> --
>> Patrick Shirkey
>> Boost Hardware Ltd
>> --
>> 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
> --
> 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
>
--
Patrick Shirkey
Boost Hardware Ltd
^ permalink raw reply
* [PATCH v3 bluetooth-next 4/4] ieee802154: 6lowpan: rename process_data and lowpan_process_data
From: Martin Townsend @ 2014-10-23 13:12 UTC (permalink / raw)
To: linux-bluetooth, linux-wpan
Cc: marcel, alex.aring, jukka.rissanen, Martin Townsend
In-Reply-To: <1414069948-29731-1-git-send-email-martin.townsend@xsilon.com>
From: Martin Townsend <mtownsend1973@gmail.com>
As we have decouple decompression from data delivery we can now rename all
occurences of process_data in receive path.
Signed-off-by: Martin Townsend <mtownsend1973@gmail.com>
---
include/net/6lowpan.h | 10 ++++++----
net/6lowpan/iphc.c | 12 +++++++-----
net/bluetooth/6lowpan.c | 15 ++++++++-------
net/ieee802154/6lowpan_rtnl.c | 15 ++++++++-------
4 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
index abfa359..dc03d77 100644
--- a/include/net/6lowpan.h
+++ b/include/net/6lowpan.h
@@ -372,10 +372,12 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 *dgram_offset)
return skb->len + uncomp_header - ret;
}
-int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
- const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
- const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
- u8 iphc0, u8 iphc1);
+int
+lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev,
+ const u8 *saddr, const u8 saddr_type,
+ const u8 saddr_len, const u8 *daddr,
+ const u8 daddr_type, const u8 daddr_len,
+ u8 iphc0, u8 iphc1);
int lowpan_header_compress(struct sk_buff *skb, struct net_device *dev,
unsigned short type, const void *_daddr,
const void *_saddr, unsigned int len);
diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 45714fe..73a7065 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -301,10 +301,12 @@ err:
/* TTL uncompression values */
static const u8 lowpan_ttl_values[] = { 0, 1, 64, 255 };
-int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
- const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
- const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
- u8 iphc0, u8 iphc1)
+int
+lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev,
+ const u8 *saddr, const u8 saddr_type,
+ const u8 saddr_len, const u8 *daddr,
+ const u8 daddr_type, const u8 daddr_len,
+ u8 iphc0, u8 iphc1)
{
struct ipv6hdr hdr = {};
u8 tmp, num_context = 0;
@@ -480,7 +482,7 @@ drop:
kfree_skb(skb);
return -EINVAL;
}
-EXPORT_SYMBOL_GPL(lowpan_process_data);
+EXPORT_SYMBOL_GPL(lowpan_header_decompress);
static u8 lowpan_compress_addr_64(u8 **hc_ptr, u8 shift,
const struct in6_addr *ipaddr,
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 40e2cec..aa6ebbf 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -257,8 +257,8 @@ static int give_skb_to_upper(struct sk_buff *skb, struct net_device *dev)
return netif_rx(skb_cp);
}
-static int process_data(struct sk_buff *skb, struct net_device *netdev,
- struct l2cap_chan *chan)
+static int iphc_decompress(struct sk_buff *skb, struct net_device *netdev,
+ struct l2cap_chan *chan)
{
const u8 *saddr, *daddr;
u8 iphc0, iphc1;
@@ -287,10 +287,11 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev,
if (lowpan_fetch_skb_u8(skb, &iphc1))
goto drop;
- return lowpan_process_data(skb, netdev,
- saddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
- daddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
- iphc0, iphc1);
+ return lowpan_header_decompress(skb, netdev,
+ saddr, IEEE802154_ADDR_LONG,
+ EUI64_ADDR_LEN, daddr,
+ IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
+ iphc0, iphc1);
drop:
kfree_skb(skb);
@@ -346,7 +347,7 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
if (!local_skb)
goto drop;
- ret = process_data(local_skb, dev, chan);
+ ret = iphc_decompress(local_skb, dev, chan);
if (ret < 0)
goto drop;
diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 05a0a84..64ec0cd 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -166,7 +166,8 @@ static int lowpan_give_skb_to_devices(struct sk_buff *skb,
return stat;
}
-static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
+static int
+iphc_decompress(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
{
u8 iphc0, iphc1;
struct ieee802154_addr_sa sa, da;
@@ -196,9 +197,9 @@ static int process_data(struct sk_buff *skb, const struct ieee802154_hdr *hdr)
else
dap = &da.hwaddr;
- return lowpan_process_data(skb, skb->dev, sap, sa.addr_type,
- IEEE802154_ADDR_LEN, dap, da.addr_type,
- IEEE802154_ADDR_LEN, iphc0, iphc1);
+ return lowpan_header_decompress(skb, skb->dev, sap, sa.addr_type,
+ IEEE802154_ADDR_LEN, dap, da.addr_type,
+ IEEE802154_ADDR_LEN, iphc0, iphc1);
drop:
kfree_skb(skb);
@@ -541,7 +542,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
} else {
switch (skb->data[0] & 0xe0) {
case LOWPAN_DISPATCH_IPHC: /* ipv6 datagram */
- ret = process_data(skb, &hdr);
+ ret = iphc_decompress(skb, &hdr);
if (ret < 0)
goto drop;
@@ -549,7 +550,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
case LOWPAN_DISPATCH_FRAG1: /* first fragment header */
ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
if (ret == 1) {
- ret = process_data(skb, &hdr);
+ ret = iphc_decompress(skb, &hdr);
if (ret < 0)
goto drop;
@@ -563,7 +564,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
case LOWPAN_DISPATCH_FRAGN: /* next fragments headers */
ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAGN);
if (ret == 1) {
- ret = process_data(skb, &hdr);
+ ret = iphc_decompress(skb, &hdr);
if (ret < 0)
goto drop;
--
1.9.1
^ permalink raw reply related
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