[PATCH 0/1] Fix incoming attrib-server connection MTUs

Linux bluetooth development
 help / color / mirror / Atom feed

* [PATCH 0/1] Fix incoming attrib-server connection MTUs
@ 2011-01-20 19:09 Brian Gix
  2011-01-20 19:09 ` [PATCH] " Brian Gix
  0 siblings, 1 reply; 3+ messages in thread
From: Brian Gix @ 2011-01-20 19:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: vinicius.gomes, claudio.takahasi, johan.hedberg, padovan

This patch fixes bug detected by Johan.

The reworked patch passes a user_data argument to the bt_io_listen
call which identifies the server IO channel (le_io or l2cap_io). This
is propegated to the eventual connect_event, and used to set up the
correct default MTU for the underlying RF carrier channel.

As stated in the previous patches, this default MTU is important when
reading long Attributes, because some/most remote GATT clients assume
completion of an Attribute read when a shorter-than-MTU result is returned,
making it important for the server to completely fill the response
packets when possible.

--
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] Fix incoming attrib-server connection MTUs
  2011-01-20 19:09 [PATCH 0/1] Fix incoming attrib-server connection MTUs Brian Gix
@ 2011-01-20 19:09 ` Brian Gix
  2011-01-21  6:50   ` Johan Hedberg
  0 siblings, 1 reply; 3+ messages in thread
From: Brian Gix @ 2011-01-20 19:09 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: vinicius.gomes, claudio.takahasi, johan.hedberg, padovan,
	Brian Gix

It is important for the Attribute Server to be aware of and
completely fill response packets up to the full MTU when
reading long attributes.  Some remote devices will only request
additional (READ_BLOB) data if the preceding read sent the
maximum amount of data.

Incoming connections are identified as L2CAP or LE by pointers
to the Service IO channel the incoming connection was recieved on
in the user_data parameter. L2CAP channels are set to the BR/EDR minimum
MTU of 48, and LE channels to the LE payload size of 23.
---
 attrib/att.h        |    3 ++-
 attrib/gatt.c       |   28 ++++++++++++++--------------
 src/attrib-server.c |   13 +++++++++----
 3 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/attrib/att.h b/attrib/att.h
index 29ab0e6..1caa62a 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -107,7 +107,8 @@
 
 
 #define ATT_MAX_MTU				256
-#define ATT_DEFAULT_MTU				23
+#define ATT_DEFAULT_L2CAP_MTU			48
+#define ATT_DEFAULT_LE_MTU			23
 
 /* Requirements for read/write operations */
 enum {
diff --git a/attrib/gatt.c b/attrib/gatt.c
index 5d7887e..f3b513e 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -105,7 +105,7 @@ static void primary_by_uuid_cb(guint8 status, const guint8 *ipdu,
 	struct discover_primary *dp = user_data;
 	GSList *ranges, *last;
 	struct att_range *range;
-	uint8_t opdu[ATT_DEFAULT_MTU];
+	uint8_t opdu[ATT_DEFAULT_LE_MTU];
 	guint16 oplen;
 	int err = 0;
 
@@ -195,7 +195,7 @@ static void primary_all_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
 	err = 0;
 
 	if (end != 0xffff) {
-		uint8_t opdu[ATT_DEFAULT_MTU];
+		uint8_t opdu[ATT_DEFAULT_LE_MTU];
 		guint16 oplen = encode_discover_primary(end + 1, 0xffff, NULL,
 							opdu, sizeof(opdu));
 
@@ -214,7 +214,7 @@ guint gatt_discover_primary(GAttrib *attrib, uuid_t *uuid, gatt_cb_t func,
 							gpointer user_data)
 {
 	struct discover_primary *dp;
-	uint8_t pdu[ATT_DEFAULT_MTU];
+	uint8_t pdu[ATT_DEFAULT_LE_MTU];
 	GAttribResultFunc cb;
 	guint16 plen;
 
@@ -245,7 +245,7 @@ static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
 	struct discover_char *dc = user_data;
 	struct att_data_list *list;
 	unsigned int i, err;
-	uint8_t opdu[ATT_DEFAULT_MTU];
+	uint8_t opdu[ATT_DEFAULT_LE_MTU];
 	guint16 oplen;
 	uuid_t uuid;
 	uint16_t last = 0;
@@ -314,7 +314,7 @@ done:
 guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
 					gatt_cb_t func, gpointer user_data)
 {
-	uint8_t pdu[ATT_DEFAULT_MTU];
+	uint8_t pdu[ATT_DEFAULT_LE_MTU];
 	struct discover_char *dc;
 	guint16 plen;
 	uuid_t uuid;
@@ -342,7 +342,7 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
 					uuid_t *uuid, GAttribResultFunc func,
 					gpointer user_data)
 {
-	uint8_t pdu[ATT_DEFAULT_MTU];
+	uint8_t pdu[ATT_DEFAULT_LE_MTU];
 	guint16 plen;
 
 	plen = enc_read_by_type_req(start, end, uuid, pdu, sizeof(pdu));
@@ -381,7 +381,7 @@ static void read_blob_helper(guint8 status, const guint8 *rpdu, guint16 rlen,
 							gpointer user_data)
 {
 	struct read_long_data *long_read = user_data;
-	uint8_t pdu[ATT_DEFAULT_MTU];
+	uint8_t pdu[ATT_DEFAULT_LE_MTU];
 	guint8 *tmp;
 	guint16 plen;
 	guint id;
@@ -402,7 +402,7 @@ static void read_blob_helper(guint8 status, const guint8 *rpdu, guint16 rlen,
 	long_read->buffer = tmp;
 	long_read->size += rlen - 1;
 
-	if (rlen < ATT_DEFAULT_MTU)
+	if (rlen < ATT_DEFAULT_LE_MTU)
 		goto done;
 
 	plen = enc_read_blob_req(long_read->handle, long_read->size - 1,
@@ -427,11 +427,11 @@ static void read_char_helper(guint8 status, const guint8 *rpdu,
 					guint16 rlen, gpointer user_data)
 {
 	struct read_long_data *long_read = user_data;
-	uint8_t pdu[ATT_DEFAULT_MTU];
+	uint8_t pdu[ATT_DEFAULT_LE_MTU];
 	guint16 plen;
 	guint id;
 
-	if (status != 0 || rlen < ATT_DEFAULT_MTU)
+	if (status != 0 || rlen < ATT_DEFAULT_LE_MTU)
 		goto done;
 
 	long_read->buffer = g_malloc(rlen);
@@ -461,7 +461,7 @@ done:
 guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
 							gpointer user_data)
 {
-	uint8_t pdu[ATT_DEFAULT_MTU];
+	uint8_t pdu[ATT_DEFAULT_LE_MTU];
 	guint16 plen;
 	guint id;
 	struct read_long_data *long_read;
@@ -493,7 +493,7 @@ guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
 guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
 			int vlen, GAttribResultFunc func, gpointer user_data)
 {
-	uint8_t pdu[ATT_DEFAULT_MTU];
+	uint8_t pdu[ATT_DEFAULT_LE_MTU];
 	guint16 plen;
 
 	plen = enc_write_req(handle, value, vlen, pdu, sizeof(pdu));
@@ -504,7 +504,7 @@ guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
 guint gatt_find_info(GAttrib *attrib, uint16_t start, uint16_t end,
 				GAttribResultFunc func, gpointer user_data)
 {
-	uint8_t pdu[ATT_DEFAULT_MTU];
+	uint8_t pdu[ATT_DEFAULT_LE_MTU];
 	guint16 plen;
 
 	plen = enc_find_info_req(start, end, pdu, sizeof(pdu));
@@ -518,7 +518,7 @@ guint gatt_find_info(GAttrib *attrib, uint16_t start, uint16_t end,
 guint gatt_write_cmd(GAttrib *attrib, uint16_t handle, uint8_t *value, int vlen,
 				GDestroyNotify notify, gpointer user_data)
 {
-	uint8_t pdu[ATT_DEFAULT_MTU];
+	uint8_t pdu[ATT_DEFAULT_LE_MTU];
 	guint16 plen;
 
 	plen = enc_write_cmd(handle, value, vlen, pdu, sizeof(pdu));
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 923bde7..f03a5b9 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -737,6 +737,7 @@ done:
 static void connect_event(GIOChannel *io, GError *err, void *user_data)
 {
 	struct gatt_channel *channel;
+	GIOChannel **server_io = user_data;
 	GError *gerr = NULL;
 
 	if (err) {
@@ -758,8 +759,12 @@ static void connect_event(GIOChannel *io, GError *err, void *user_data)
 		return;
 	}
 
+	if (server_io == &l2cap_io)
+		channel->mtu = ATT_DEFAULT_L2CAP_MTU;
+	else
+		channel->mtu = ATT_DEFAULT_LE_MTU;
+
 	channel->attrib = g_attrib_new(io);
-	channel->mtu = ATT_DEFAULT_MTU;
 	g_io_channel_unref(io);
 
 	channel->id = g_attrib_register(channel->attrib, GATTRIB_ALL_EVENTS,
@@ -775,7 +780,7 @@ static void confirm_event(GIOChannel *io, void *user_data)
 {
 	GError *gerr = NULL;
 
-	if (bt_io_accept(io, connect_event, NULL, NULL, &gerr) == FALSE) {
+	if (bt_io_accept(io, connect_event, user_data, NULL, &gerr) == FALSE) {
 		error("bt_io_accept: %s", gerr->message);
 		g_error_free(gerr);
 		g_io_channel_unref(io);
@@ -824,7 +829,7 @@ int attrib_server_init(void)
 
 	/* BR/EDR socket */
 	l2cap_io = bt_io_listen(BT_IO_L2CAP, NULL, confirm_event,
-					NULL, NULL, &gerr,
+					&l2cap_io, NULL, &gerr,
 					BT_IO_OPT_SOURCE_BDADDR, BDADDR_ANY,
 					BT_IO_OPT_PSM, GATT_PSM,
 					BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
@@ -857,7 +862,7 @@ int attrib_server_init(void)
 
 	/* LE socket */
 	le_io = bt_io_listen(BT_IO_L2CAP, NULL, confirm_event,
-					NULL, NULL, &gerr,
+					&le_io, NULL, &gerr,
 					BT_IO_OPT_SOURCE_BDADDR, BDADDR_ANY,
 					BT_IO_OPT_CID, GATT_CID,
 					BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
-- 
1.7.1
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix incoming attrib-server connection MTUs
  2011-01-20 19:09 ` [PATCH] " Brian Gix
@ 2011-01-21  6:50   ` Johan Hedberg
  0 siblings, 0 replies; 3+ messages in thread
From: Johan Hedberg @ 2011-01-21  6:50 UTC (permalink / raw)
  To: Brian Gix; +Cc: linux-bluetooth, vinicius.gomes, claudio.takahasi, padovan

Hi Brian,

On Thu, Jan 20, 2011, Brian Gix wrote:
> It is important for the Attribute Server to be aware of and completely
> fill response packets up to the full MTU when reading long attributes.
> Some remote devices will only request additional (READ_BLOB) data if
> the preceding read sent the maximum amount of data.
> 
> Incoming connections are identified as L2CAP or LE by pointers to the
> Service IO channel the incoming connection was recieved on in the
> user_data parameter. L2CAP channels are set to the BR/EDR minimum MTU
> of 48, and LE channels to the LE payload size of 23.
> ---
>  attrib/att.h        |    3 ++-
>  attrib/gatt.c       |   28 ++++++++++++++--------------
>  src/attrib-server.c |   13 +++++++++----
>  3 files changed, 25 insertions(+), 19 deletions(-)

Thanks for the updated patch. It looks good now and has been pushed
upstream.

Johan

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-01-21  6:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-20 19:09 [PATCH 0/1] Fix incoming attrib-server connection MTUs Brian Gix
2011-01-20 19:09 ` [PATCH] " Brian Gix
2011-01-21  6:50   ` Johan Hedberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox