Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH bluetooth] Bluetooth: Fix missing channel unlock in l2cap_le_credits
From: Jukka Rissanen @ 2014-10-14  8:31 UTC (permalink / raw)
  To: Martin Townsend; +Cc: linux-bluetooth, marcel, johan.hedberg
In-Reply-To: <1413224685-3700-1-git-send-email-mtownsend1973@gmail.com>

Hi,

On ma, 2014-10-13 at 19:24 +0100, Martin Townsend wrote:
> In the error case where credits is greater than max_credits there
> is a missing l2cap_chan_unlock before returning.
> 
> Signed-off-by: Martin Townsend <mtownsend1973@gmail.com>
> ---
>  net/bluetooth/l2cap_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 46547b9..bfb6af8 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -5544,6 +5544,7 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn,
>  	if (credits > max_credits) {
>  		BT_ERR("LE credits overflow");
>  		l2cap_send_disconn_req(chan, ECONNRESET);
> +		l2cap_chan_unlock(chan);
>  
>  		/* Return 0 so that we don't trigger an unnecessary
>  		 * command reject packet.

I did some testing with this patch and although it did not fix the
inconsistent lock issue I am seeing, it did fix the mutex hang. I have
two locking issue and this patch fixed the other one.

Tested-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>


Cheers,
Jukka

^ permalink raw reply

* Not getting an AVDTP: incoming connect
From: John Tobias @ 2014-10-14  0:09 UTC (permalink / raw)
  To: linux-bluetooth@vger.kernel.org

Hi Marcell / Bluez Team:

I was running the PTS 5.2 and there were an occurrences that the bluez
did not show a similar message below:

[agent] Authorize service 0000110d-0000-1000-8000-00805f9b34fb (yes/no):


1. I logs below are the traces of the bluetoothd not prompting the
Authorize service message:

t 13 23:27:22 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/adapter.c:connected_callback() hci0 device 00:1B:DC:07:32:D3
connected eir_len 31
Oct 13 23:27:22 DEV9MRCNERGAAAQYDKGP bluetoothd[152]: on_connected
Oct 13 23:27:26 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/adapter.c:dev_disconnected() Device 00:1B:DC:07:32:D3
disconnected, reason 2
Oct 13 23:27:26 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/adapter.c:adapter_remove_connection()
Oct 13 23:27:26 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
plugins/policy.c:disconnect_cb() reason 2
Oct 13 23:27:26 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/adapter.c:bonding_attempt_complete() hci0 bdaddr 00:1B:DC:07:32:D3
type 0 status 0xe
Oct 13 23:27:26 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/device.c:device_bonding_complete() bonding (nil) status 0x0e
Oct 13 23:27:26 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/device.c:device_bonding_failed() status 14
Oct 13 23:27:26 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/adapter.c:resume_discovery()
Oct 13 23:27:26 DEV9MRCNERGAAAQYDKGP bluetoothd[152]: on_disconnected
Oct 13 23:27:26 DEV9MRCNERGAAAQYDKGP bluetoothd[152]: setting adv
params failed: Operation not permitted
Oct 13 23:27:26 DEV9MRCNERGAAAQYDKGP bluetoothd[152]: failed to enable
advertising


2. These messages are the traces of the bluetoothd showing the
Authorize service message:

Oct 13 23:29:48 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/adapter.c:connected_callback() hci0 device 00:1B:DC:07:32:D3
connected eir_len 31
Oct 13 23:29:48 DEV9MRCNERGAAAQYDKGP bluetoothd[152]: on_connected
Oct 13 23:29:48 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_confirm_cb() AVDTP: incoming connect from
00:1B:DC:07:32:D3
Oct 13 23:29:48 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/source.c:source_set_state() State changed
/org/bluez/hci0/dev_00_1B_DC_07_32_D3: SOURCE_STATE_DISCONNECTED ->
SOURCE_STATE_CONNECTING
Oct 13 23:29:48 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/agent.c:agent_ref() 0x77a601d0: ref=2
Oct 13 23:29:48 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/agent.c:agent_authorize_service() authorize service request was
sent for /org/bluez/hci0/dev_00_1B_DC_07_32_D3
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/agent.c:agent_ref() 0x77a601d0: ref=3
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/agent.c:agent_unref() 0x77a601d0: ref=2
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/agent.c:agent_unref() 0x77a601d0: ref=1
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_connect_cb() AVDTP: connected signaling
channel to 00:1B:DC:07:32:D3
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_connect_cb() AVDTP imtu=672, omtu=672
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:session_cb()
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_parse_cmd() Received DISCOVER_CMD
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:session_cb()
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_parse_cmd() Received
GET_CAPABILITIES_CMD
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/a2dp.c:endpoint_getcap_ind() Sink 0x77a69bd8:
Get_Capability_Ind
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:session_cb()
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_parse_cmd() Received
SET_CONFIGURATION_CMD
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/a2dp.c:endpoint_setconf_ind() Sink 0x77a69bd8:
Set_Configuration_Ind
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_ref() 0x77a6afa8: ref=1
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/a2dp.c:setup_ref() 0x77a5d040: ref=1
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/media.c:media_endpoint_async_call() Calling
SetConfiguration: name = :1.6 path = /MediaEndpoint/A2DPSink
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_ref() 0x77a6afa8: ref=2
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_sep_set_state() stream state changed:
IDLE -> CONFIGURED
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/a2dp.c:setup_unref() 0x77a5d040: ref=0
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/a2dp.c:setup_free() 0x77a5d040
Oct 13 23:29:52 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_unref() 0x77a6afa8: ref=1
Oct 13 23:29:53 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:session_cb()
Oct 13 23:29:53 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_parse_cmd() Received OPEN_CMD
Oct 13 23:29:53 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/a2dp.c:open_ind() Sink 0x77a69bd8: Open_Ind
Oct 13 23:29:53 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_confirm_cb() AVDTP: incoming connect from
00:1B:DC:07:32:D3
Oct 13 23:29:53 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_connect_cb() AVDTP: connected transport
channel to 00:1B:DC:07:32:D3
Oct 13 23:29:53 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/avdtp.c:avdtp_sep_set_state() stream state changed:
CONFIGURED -> OPEN
Oct 13 23:29:53 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
src/service.c:change_state() 0x77a70eb8: device 00:1B:DC:07:32:D3
profile a2dp-source state changed: disconnected -> connected (0)
Oct 13 23:29:53 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
plugins/policy.c:service_cb() Added a2dp-source reconnect 1
Oct 13 23:29:53 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/source.c:source_set_state() State changed
/org/bluez/hci0/dev_00_1B_DC_07_32_D3: SOURCE_STATE_CONNECTING ->
SOURCE_STATE_CONNECTED
Oct 13 23:29:53 DEV9MRCNERGAAAQYDKGP bluetoothd[152]:
profiles/audio/transport.c:transport_update_playing()
/org/bluez/hci0/dev_00_1B_DC_07_32_D3/fd19 State=TRANSPORT_STATE_IDLE


It seems the avdtp_server_socket did not received the signals and
that's the reason why the avdtp_confirm_cb function did not execute. I
would like to know what are the possible scenario why the bluetoothd
did not receive the event?.
Because, if I re-run the PTS with the same test case, the bluez could
received the event.

Regards,

john

^ permalink raw reply

* [PATCH v2 5/5] shared/gatt-server: Support Read By Group Type request.
From: Arman Uguray @ 2014-10-13 21:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray
In-Reply-To: <1413234602-20566-1-git-send-email-armansito@chromium.org>

This patch adds handling for the Read By Group Type request.
---
 src/shared/gatt-server.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 178 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index c7b0873..d6d0155 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -45,6 +45,7 @@ struct bt_gatt_server {
 	uint16_t mtu;
 
 	unsigned int mtu_id;
+	unsigned int read_by_grp_type_id;
 
 	bt_gatt_server_debug_func_t debug_callback;
 	bt_gatt_server_destroy_func_t debug_destroy;
@@ -54,6 +55,7 @@ struct bt_gatt_server {
 static void gatt_server_unregister_handlers(struct bt_gatt_server *server)
 {
 	bt_att_unregister(server->att, server->mtu_id);
+	bt_att_unregister(server->att, server->read_by_grp_type_id);
 }
 
 static void gatt_server_cleanup(struct bt_gatt_server *server)
@@ -71,6 +73,166 @@ static void encode_error_rsp(uint8_t opcode, uint16_t handle, uint8_t ecode,
 	put_le16(handle, pdu + 1);
 }
 
+static bool get_uuid_le(const uint8_t *uuid, size_t len, bt_uuid_t *out_uuid)
+{
+	uint128_t u128;
+
+	switch (len) {
+	case 2:
+		bt_uuid16_create(out_uuid, get_le16(uuid));
+		return true;
+	case 16:
+		bswap_128(uuid, &u128.data);
+		bt_uuid128_create(out_uuid, u128);
+		return true;
+	default:
+		return false;
+	}
+
+	return false;
+}
+
+static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
+						uint16_t mtu,
+						uint8_t *pdu, uint16_t *len)
+{
+	int iter = 0;
+	uint16_t start_handle, end_handle;
+	uint8_t *value;
+	int value_len;
+	uint8_t data_val_len;
+
+	*len = 0;
+
+	while (queue_peek_head(q)) {
+		start_handle = PTR_TO_UINT(queue_pop_head(q));
+		value = NULL;
+		value_len = 0;
+
+		/* This should never be deferred to the read callback for
+		 * primary/secondary service declarations.
+		 */
+		if (!gatt_db_read(db, start_handle, 0,
+						BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
+						NULL, &value,
+						&value_len) || value_len < 0)
+			return false;
+
+		/* Use the first attribute to determine the length of each
+		 * attribute data unit. Stop the list when a different attribute
+		 * value is seen.
+		 */
+		if (iter == 0) {
+			data_val_len = value_len;
+			pdu[0] = data_val_len + 4;
+			iter++;
+		} else if (value_len != data_val_len)
+			break;
+
+		/* Stop if this unit would surpass the MTU */
+		if (iter + data_val_len + 4 > mtu)
+			break;
+
+		end_handle = gatt_db_get_end_handle(db, start_handle);
+
+		put_le16(start_handle, pdu + iter);
+		put_le16(end_handle, pdu + iter + 2);
+		memcpy(pdu + iter + 4, value, value_len);
+
+		iter += data_val_len + 4;
+	}
+
+	*len = iter;
+
+	return true;
+}
+
+static void read_by_grp_type_cb(uint8_t opcode, const void *pdu,
+					uint16_t length, void *user_data)
+{
+	struct bt_gatt_server *server = user_data;
+	uint16_t start, end;
+	bt_uuid_t type;
+	bt_uuid_t prim, snd;
+	uint16_t mtu = bt_att_get_mtu(server->att);
+	uint8_t rsp_pdu[mtu];
+	uint16_t rsp_len;
+	uint8_t rsp_opcode;
+	uint8_t ecode = 0;
+	uint16_t ehandle = 0;
+	struct queue *q = NULL;
+
+	if (length != 6 && length != 20) {
+		ecode = BT_ATT_ERROR_INVALID_PDU;
+		goto error;
+	}
+
+	q = queue_new();
+	if (!q) {
+		ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
+		goto error;
+	}
+
+	start = get_le16(pdu);
+	end = get_le16(pdu + 2);
+	get_uuid_le(pdu + 4, length - 4, &type);
+
+	util_debug(server->debug_callback, server->debug_data,
+				"Read By Grp Type - start: 0x%04x end: 0x%04x",
+				start, end);
+
+	if (!start || !end) {
+		ecode = BT_ATT_ERROR_INVALID_HANDLE;
+		goto error;
+	}
+
+	ehandle = start;
+
+	if (start > end) {
+		ecode = BT_ATT_ERROR_INVALID_HANDLE;
+		goto error;
+	}
+
+	/* GATT defines that only the <<Primary Service>> and
+	 * <<Secondary Service>> group types can be used for the
+	 * "Read By Group Type" request (Core v4.1, Vol 3, sec 2.5.3). Return an
+	 * error if any other group type is given.
+	 */
+	bt_uuid16_create(&prim, GATT_PRIM_SVC_UUID);
+	bt_uuid16_create(&snd, GATT_SND_SVC_UUID);
+	if (bt_uuid_cmp(&type, &prim) && bt_uuid_cmp(&type, &snd)) {
+		ecode = BT_ATT_ERROR_UNSUPPORTED_GROUP_TYPE;
+		goto error;
+	}
+
+	gatt_db_read_by_group_type(server->db, start, end, type, q);
+
+	if (queue_isempty(q)) {
+		ecode = BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND;
+		goto error;
+	}
+
+	if (!encode_read_by_grp_type_rsp(server->db, q, mtu, rsp_pdu,
+								&rsp_len)) {
+		ecode = BT_ATT_ERROR_UNLIKELY;
+		goto error;
+	}
+
+	rsp_opcode = BT_ATT_OP_READ_BY_GRP_TYPE_RSP;
+
+	goto done;
+
+error:
+	rsp_opcode = BT_ATT_OP_ERROR_RSP;
+	rsp_len = 4;
+	encode_error_rsp(opcode, ehandle, ecode, rsp_pdu);
+
+done:
+	queue_destroy(q, NULL);
+	bt_att_send(server->att, rsp_opcode, rsp_pdu, rsp_len,
+							NULL, NULL, NULL);
+}
+
 static void exchange_mtu_cb(uint8_t opcode, const void *pdu,
 					uint16_t length, void *user_data)
 {
@@ -97,18 +259,33 @@ static void exchange_mtu_cb(uint8_t opcode, const void *pdu,
 	/* Set MTU to be the minimum */
 	server->mtu = final_mtu;
 	bt_att_set_mtu(server->att, final_mtu);
+
+	util_debug(server->debug_callback, server->debug_data,
+			"MTU exchange complete, with MTU: %u", final_mtu);
 }
 
 static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
 {
-	/* EXCHANGE MTU request */
+	/* Exchange MTU */
 	server->mtu_id = bt_att_register(server->att, BT_ATT_OP_MTU_REQ,
 								exchange_mtu_cb,
 								server, NULL);
 	if (!server->mtu_id)
 		return false;
 
+	/* Read By Group Type */
+	server->read_by_grp_type_id = bt_att_register(server->att,
+						BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
+						read_by_grp_type_cb,
+						server, NULL);
+	if (!server->read_by_grp_type_id)
+		goto fail;
+
 	return true;
+
+fail:
+	gatt_server_unregister_handlers(server);
+	return false;
 }
 
 struct bt_gatt_server *bt_gatt_server_new(struct gatt_db *db,
-- 
2.1.0.rc2.206.gedb03e5


^ permalink raw reply related

* [PATCH v2 4/5] shared/gatt-server: Support Exchange MTU request.
From: Arman Uguray @ 2014-10-13 21:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray
In-Reply-To: <1413234602-20566-1-git-send-email-armansito@chromium.org>

This patch adds handling for the exchange MTU request.
---
 src/shared/gatt-server.c | 60 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 57 insertions(+), 3 deletions(-)

diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 544e60e..c7b0873 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -34,29 +34,83 @@
 #define MAX(a, b) ((a) > (b) ? (a) : (b))
 #endif
 
+#ifndef MIN
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
 struct bt_gatt_server {
 	struct gatt_db *db;
 	struct bt_att *att;
 	int ref_count;
 	uint16_t mtu;
 
+	unsigned int mtu_id;
+
 	bt_gatt_server_debug_func_t debug_callback;
 	bt_gatt_server_destroy_func_t debug_destroy;
 	void *debug_data;
 };
 
-static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
+static void gatt_server_unregister_handlers(struct bt_gatt_server *server)
 {
-	/* TODO */
-	return true;
+	bt_att_unregister(server->att, server->mtu_id);
 }
 
 static void gatt_server_cleanup(struct bt_gatt_server *server)
 {
+	gatt_server_unregister_handlers(server);
 	bt_att_unref(server->att);
 	server->att = NULL;
 }
 
+static void encode_error_rsp(uint8_t opcode, uint16_t handle, uint8_t ecode,
+								uint8_t pdu[4])
+{
+	pdu[0] = opcode;
+	pdu[3] = ecode;
+	put_le16(handle, pdu + 1);
+}
+
+static void exchange_mtu_cb(uint8_t opcode, const void *pdu,
+					uint16_t length, void *user_data)
+{
+	struct bt_gatt_server *server = user_data;
+	uint16_t client_rx_mtu;
+	uint16_t final_mtu;
+	uint8_t rsp_pdu[4];
+
+	if (length != 2) {
+		encode_error_rsp(opcode, 0, BT_ATT_ERROR_INVALID_PDU, rsp_pdu);
+		bt_att_send(server->att, BT_ATT_OP_ERROR_RSP, rsp_pdu,
+					sizeof(rsp_pdu), NULL, NULL, NULL);
+		return;
+	}
+
+	client_rx_mtu = get_le16(pdu);
+	final_mtu = MAX(MIN(client_rx_mtu, server->mtu), BT_ATT_DEFAULT_LE_MTU);
+
+	/* Respond with the server MTU */
+	put_le16(server->mtu, rsp_pdu);
+	bt_att_send(server->att, BT_ATT_OP_MTU_RSP, rsp_pdu, 2, NULL, NULL,
+									NULL);
+
+	/* Set MTU to be the minimum */
+	server->mtu = final_mtu;
+	bt_att_set_mtu(server->att, final_mtu);
+}
+
+static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
+{
+	/* EXCHANGE MTU request */
+	server->mtu_id = bt_att_register(server->att, BT_ATT_OP_MTU_REQ,
+								exchange_mtu_cb,
+								server, NULL);
+	if (!server->mtu_id)
+		return false;
+
+	return true;
+}
+
 struct bt_gatt_server *bt_gatt_server_new(struct gatt_db *db,
 					struct bt_att *att, uint16_t mtu)
 {
-- 
2.1.0.rc2.206.gedb03e5


^ permalink raw reply related

* [PATCH v2 3/5] shared/gatt-server: Introduce shared/gatt-server.
From: Arman Uguray @ 2014-10-13 21:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray
In-Reply-To: <1413234602-20566-1-git-send-email-armansito@chromium.org>

This patch introduces bt_gatt_server which will implement the server-side of the
ATT protocol over a bt_att structure and construct responses based on a gatt_db
structure.
---
 Makefile.am              |   1 +
 src/shared/gatt-server.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/gatt-server.h |  40 +++++++++++++++
 3 files changed, 166 insertions(+)
 create mode 100644 src/shared/gatt-server.c
 create mode 100644 src/shared/gatt-server.h

diff --git a/Makefile.am b/Makefile.am
index 1a1a43f..2dfea28 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -114,6 +114,7 @@ shared_sources = src/shared/io.h src/shared/timeout.h \
 			src/shared/att.h src/shared/att.c \
 			src/shared/gatt-helpers.h src/shared/gatt-helpers.c \
 			src/shared/gatt-client.h src/shared/gatt-client.c \
+			src/shared/gatt-server.h src/shared/gatt-server.c \
 			src/shared/gatt-db.h src/shared/gatt-db.c \
 			src/shared/gap.h src/shared/gap.c
 
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
new file mode 100644
index 0000000..544e60e
--- /dev/null
+++ b/src/shared/gatt-server.c
@@ -0,0 +1,125 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Google Inc.
+ *
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library 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
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include "src/shared/att.h"
+#include "lib/uuid.h"
+#include "src/shared/queue.h"
+#include "src/shared/gatt-db.h"
+#include "src/shared/gatt-server.h"
+#include "src/shared/gatt-helpers.h"
+#include "src/shared/att-types.h"
+#include "src/shared/util.h"
+
+#ifndef MAX
+#define MAX(a, b) ((a) > (b) ? (a) : (b))
+#endif
+
+struct bt_gatt_server {
+	struct gatt_db *db;
+	struct bt_att *att;
+	int ref_count;
+	uint16_t mtu;
+
+	bt_gatt_server_debug_func_t debug_callback;
+	bt_gatt_server_destroy_func_t debug_destroy;
+	void *debug_data;
+};
+
+static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
+{
+	/* TODO */
+	return true;
+}
+
+static void gatt_server_cleanup(struct bt_gatt_server *server)
+{
+	bt_att_unref(server->att);
+	server->att = NULL;
+}
+
+struct bt_gatt_server *bt_gatt_server_new(struct gatt_db *db,
+					struct bt_att *att, uint16_t mtu)
+{
+	struct bt_gatt_server *server;
+
+	if (!att)
+		return NULL;
+
+	server = new0(struct bt_gatt_server, 1);
+	if (!server)
+		return NULL;
+
+	server->db = db;
+	server->att = bt_att_ref(att);
+	server->mtu = MAX(mtu, BT_ATT_DEFAULT_LE_MTU);
+
+	if (!gatt_server_register_att_handlers(server)) {
+		gatt_db_destroy(server->db);
+		bt_att_unref(att);
+		free(server);
+		return NULL;
+	}
+
+	return bt_gatt_server_ref(server);
+}
+
+struct bt_gatt_server *bt_gatt_server_ref(struct bt_gatt_server *server)
+{
+	if (!server)
+		return NULL;
+
+	__sync_fetch_and_add(&server->ref_count, 1);
+
+	return server;
+}
+
+void bt_gatt_server_unref(struct bt_gatt_server *server)
+{
+	if (__sync_sub_and_fetch(&server->ref_count, 1))
+		return;
+
+	if (server->debug_destroy)
+		server->debug_destroy(server->debug_data);
+
+	gatt_server_cleanup(server);
+
+	free(server);
+}
+
+bool bt_gatt_server_set_debug(struct bt_gatt_server *server,
+					bt_gatt_server_debug_func_t callback,
+					void *user_data,
+					bt_gatt_server_destroy_func_t destroy)
+{
+	if (!server)
+		return false;
+
+	if (server->debug_destroy)
+		server->debug_destroy(server->debug_data);
+
+	server->debug_callback = callback;
+	server->debug_destroy = destroy;
+	server->debug_data = user_data;
+
+	return true;
+}
diff --git a/src/shared/gatt-server.h b/src/shared/gatt-server.h
new file mode 100644
index 0000000..e3c4def
--- /dev/null
+++ b/src/shared/gatt-server.h
@@ -0,0 +1,40 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Google Inc.
+ *
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library 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
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include <stdint.h>
+
+struct bt_gatt_server;
+
+struct bt_gatt_server *bt_gatt_server_new(struct gatt_db *db,
+					struct bt_att *att, uint16_t mtu);
+
+struct bt_gatt_server *bt_gatt_server_ref(struct bt_gatt_server *server);
+void bt_gatt_server_unref(struct bt_gatt_server *server);
+
+typedef void (*bt_gatt_server_destroy_func_t)(void *user_data);
+typedef void (*bt_gatt_server_debug_func_t)(const char *str, void *user_data);
+
+bool bt_gatt_server_set_debug(struct bt_gatt_server *server,
+					bt_gatt_server_debug_func_t callback,
+					void *user_data,
+					bt_gatt_server_destroy_func_t destroy);
-- 
2.1.0.rc2.206.gedb03e5


^ permalink raw reply related

* [PATCH v2 2/5] shared/att: Respond with ERROR_REQUEST_NOT_SUPPORTED for unhandled requests.
From: Arman Uguray @ 2014-10-13 21:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray
In-Reply-To: <1413234602-20566-1-git-send-email-armansito@chromium.org>

With this patch bt_att automatically responds with ERROR_REQUEST_NOT_SUPPORTED
to an incoming request for which no handler has been registered via
bt_att_register.
---
 src/shared/att.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index 96f34a3..d086ca8 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -562,6 +562,7 @@ struct notify_data {
 	uint8_t opcode;
 	uint8_t *pdu;
 	ssize_t pdu_len;
+	bool handler_found;
 };
 
 static void notify_handler(void *data, void *user_data)
@@ -575,11 +576,27 @@ static void notify_handler(void *data, void *user_data)
 	if (notify->opcode != not_data->opcode)
 		return;
 
+	not_data->handler_found = true;
+
 	if (notify->callback)
 		notify->callback(not_data->opcode, not_data->pdu,
 					not_data->pdu_len, notify->user_data);
 }
 
+static void respond_not_supported(struct bt_att *att, uint8_t opcode)
+{
+	uint8_t pdu[4];
+
+	memset(pdu, 0, sizeof(pdu));
+	pdu[0] = opcode;
+	pdu[1] = 0;
+	pdu[2] = 0;
+	pdu[3] = BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+
+	bt_att_send(att, BT_ATT_OP_ERROR_RSP, pdu, sizeof(pdu), NULL, NULL,
+									NULL);
+}
+
 static void handle_notify(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 								ssize_t pdu_len)
 {
@@ -607,6 +624,12 @@ static void handle_notify(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	}
 
 	bt_att_unref(att);
+
+	/* If this was a request and no handler was registered for it, respond
+	 * with "Not Supported"
+	 */
+	if (!data.handler_found && get_op_type(opcode) == ATT_OP_TYPE_REQ)
+		respond_not_supported(att, opcode);
 }
 
 static void disconn_handler(void *data, void *user_data)
-- 
2.1.0.rc2.206.gedb03e5


^ permalink raw reply related

* [PATCH v2 1/5] shared/att: Drop the connection if a request is received while one is pending.
From: Arman Uguray @ 2014-10-13 21:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray
In-Reply-To: <1413234602-20566-1-git-send-email-armansito@chromium.org>

With this patch, when bt_att is being used for the server role, it now makes
sure that a second request drops the connection unless a response for a previous
request has been sent.
---
 src/shared/att.c | 101 +++++++++++++++++++++++++++++++++----------------------
 1 file changed, 61 insertions(+), 40 deletions(-)

diff --git a/src/shared/att.c b/src/shared/att.c
index de35aef..96f34a3 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -64,6 +64,8 @@ struct bt_att {
 	bool in_disconn;
 	bool need_disconn_cleanup;
 
+	bool in_req;			/* There's a pending incoming request */
+
 	uint8_t *buf;
 	uint16_t mtu;
 
@@ -460,6 +462,9 @@ static bool can_write_data(struct io *io, void *user_data)
 	case ATT_OP_TYPE_IND:
 		att->pending_ind = op;
 		break;
+	case ATT_OP_TYPE_RSP:
+		/* Set in_req to false to indicate that no request is pending */
+		att->in_req = false;
 	default:
 		destroy_att_send_op(op);
 		return true;
@@ -604,6 +609,46 @@ static void handle_notify(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	bt_att_unref(att);
 }
 
+static void disconn_handler(void *data, void *user_data)
+{
+	struct att_disconn *disconn = data;
+
+	if (disconn->removed)
+		return;
+
+	if (disconn->callback)
+		disconn->callback(disconn->user_data);
+}
+
+static bool disconnect_cb(struct io *io, void *user_data)
+{
+	struct bt_att *att = user_data;
+
+	io_destroy(att->io);
+	att->io = NULL;
+
+	util_debug(att->debug_callback, att->debug_data,
+						"Physical link disconnected");
+
+	bt_att_ref(att);
+	att->in_disconn = true;
+	queue_foreach(att->disconn_list, disconn_handler, NULL);
+	att->in_disconn = false;
+
+	if (att->need_disconn_cleanup) {
+		queue_remove_all(att->disconn_list, match_disconn_removed, NULL,
+							destroy_att_disconn);
+		att->need_disconn_cleanup = false;
+	}
+
+	bt_att_cancel_all(att);
+	bt_att_unregister_all(att);
+
+	bt_att_unref(att);
+
+	return false;
+}
+
 static bool can_read_data(struct io *io, void *user_data)
 {
 	struct bt_att *att = user_data;
@@ -635,6 +680,22 @@ static bool can_read_data(struct io *io, void *user_data)
 		util_debug(att->debug_callback, att->debug_data,
 				"ATT opcode cannot be handled: 0x%02x", opcode);
 		break;
+	case ATT_OP_TYPE_REQ:
+		/* If a request is currently pending, then the sequential
+		 * protocol was violated. Disconnect the bearer and notify the
+		 * upper-layer.
+		 */
+		if (att->in_req) {
+			util_debug(att->debug_callback, att->debug_data,
+					"Received request while another is "
+					"pending: 0x%02x", opcode);
+			disconnect_cb(att->io, att);
+			return false;
+		}
+
+		att->in_req = true;
+
+		/* Fall through to the next case */
 	default:
 		/* For all other opcodes notify the upper layer of the PDU and
 		 * let them act on it.
@@ -648,46 +709,6 @@ static bool can_read_data(struct io *io, void *user_data)
 	return true;
 }
 
-static void disconn_handler(void *data, void *user_data)
-{
-	struct att_disconn *disconn = data;
-
-	if (disconn->removed)
-		return;
-
-	if (disconn->callback)
-		disconn->callback(disconn->user_data);
-}
-
-static bool disconnect_cb(struct io *io, void *user_data)
-{
-	struct bt_att *att = user_data;
-
-	io_destroy(att->io);
-	att->io = NULL;
-
-	util_debug(att->debug_callback, att->debug_data,
-						"Physical link disconnected");
-
-	bt_att_ref(att);
-	att->in_disconn = true;
-	queue_foreach(att->disconn_list, disconn_handler, NULL);
-	att->in_disconn = false;
-
-	if (att->need_disconn_cleanup) {
-		queue_remove_all(att->disconn_list, match_disconn_removed, NULL,
-							destroy_att_disconn);
-		att->need_disconn_cleanup = false;
-	}
-
-	bt_att_cancel_all(att);
-	bt_att_unregister_all(att);
-
-	bt_att_unref(att);
-
-	return false;
-}
-
 struct bt_att *bt_att_new(int fd)
 {
 	struct bt_att *att;
-- 
2.1.0.rc2.206.gedb03e5


^ permalink raw reply related

* [PATCH v2 0/5] Introduce shared/gatt-server.
From: Arman Uguray @ 2014-10-13 21:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

*v2:
  - Split read_by_grp_type_cb into two functions by moving the response PDU
    encoding loop into a helper function.

*v1:
  - Make gatt-db external to gatt-server, as initially discussed.
  - Also implement Read By Group Type request.

Arman Uguray (5):
  shared/att: Drop the connection if a request is received while one is
    pending.
  shared/att: Respond with ERROR_REQUEST_NOT_SUPPORTED for unhandled
    requests.
  shared/gatt-server: Introduce shared/gatt-server.
  shared/gatt-server: Support Exchange MTU request.
  shared/gatt-server: Support Read By Group Type request.

 Makefile.am              |   1 +
 src/shared/att.c         | 124 +++++++++++------
 src/shared/gatt-server.c | 356 +++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/gatt-server.h |  40 ++++++
 4 files changed, 481 insertions(+), 40 deletions(-)
 create mode 100644 src/shared/gatt-server.c
 create mode 100644 src/shared/gatt-server.h

-- 
2.1.0.rc2.206.gedb03e5


^ permalink raw reply

* Re: [PATCH BlueZ v1 4/4] shared/gatt-server: Support Read By Group Type request.
From: Arman Uguray @ 2014-10-13 20:57 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: Arman Uguray, linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZ+Nx9iWAYEJiZY5CSE8x_ynjRFpfawB5c-6+eCKNOHcwg@mail.gmail.com>

Hi Luiz,


>> +       if (queue_isempty(q)) {
>> +               ecode = BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND;
>> +               goto error;
>> +       }
>
> This function is quite long, perhaps using queue_foreach be better
> than iterating inline since you destroy the queue at end but it would
> be a problem if one of items fails to be encoded, the funny thing is
> that it is possible to do the same with queue_find and return true to
> interrupt in case of failure but it would be a obvious abuse of the
> API so perhaps queue_foreach_func_t could have a return as well and in
> case it return false we stop.
>

I would have to modify too many places that use queue_foreach by
adding a somewhat suspicious "return true" with no obvious meaning, so
I decided not to do that for this patch set. For v2 I went ahead and
broke the encoding bit into its own helper which I plan to extend in
the future for other read requests. Hopefully this'll make the code
more readable although it doesn't call queue_foreach.

Cheers,
Arman

^ permalink raw reply

* Re: [PATCH 00/10] android/map-client: Add MAP client daemon implementation
From: Szymon Janc @ 2014-10-13 20:47 UTC (permalink / raw)
  To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth
In-Reply-To: <1412858714-2845-1-git-send-email-grzegorz.kolodziejczyk@tieto.com>

Hi Grzegorz,

On Thursday 09 October 2014 14:45:04 Grzegorz Kolodziejczyk wrote:
> v1.
> *Add MAP client daemon implementation (skeleton, body)
> *Add haltest MAP client support
> 
> Aleksander Drewnicki (4):
>   android/client: Add skeleton for mce calls
>   android/client: Add code for mce callback
>   android/client: Add code for mce method
>   android/client: Add completion for mce method
> 
> Grzegorz Kolodziejczyk (6):
>   android/map-client: Add initial files
>   android/map-client: Add stubs for MAP client commands handlers
>   android/map-client: Add support for get remote MAS instances
>   android/ipc-tester: Add missing service opcode boundries test cases
>   android/ipc-tester: Add case for MAP client service opcode boundries
>   android/ipc-tester: Add cases for MAP client msg size
> 
>  android/Android.mk       |   5 +-
>  android/Makefile.am      |   2 +
>  android/client/haltest.c |   2 +
>  android/client/if-bt.c   |   3 +
>  android/client/if-main.h |   3 +
>  android/client/if-mce.c  |  87 +++++++++++++++++++++
>  android/ipc-tester.c     |  32 ++++++++
>  android/main.c           |  12 +++
>  android/map-client.c     | 197
> +++++++++++++++++++++++++++++++++++++++++++++++ android/map-client.h     | 
> 26 +++++++
>  10 files changed, 368 insertions(+), 1 deletion(-)
>  create mode 100644 android/client/if-mce.c
>  create mode 100644 android/map-client.c
>  create mode 100644 android/map-client.h

android/client patches are now upstream so no need to resend them. Thanks.

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCH 04/10] android/ipc-tester: Add missing service opcode boundries test cases
From: Szymon Janc @ 2014-10-13 20:32 UTC (permalink / raw)
  To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth
In-Reply-To: <1412858714-2845-5-git-send-email-grzegorz.kolodziejczyk@tieto.com>

Hi Grzegorz,

On Thursday 09 October 2014 14:45:08 Grzegorz Kolodziejczyk wrote:
> This patch adds tests sending out of range opcode for each service.
> ---
>  android/ipc-tester.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/android/ipc-tester.c b/android/ipc-tester.c
> index ea71c8d..161777d 100644
> --- a/android/ipc-tester.c
> +++ b/android/ipc-tester.c
> @@ -919,9 +919,23 @@ int main(int argc, char *argv[])
>  	test_opcode_valid("PAN", HAL_SERVICE_ID_PAN, 0x05, 0,
>  			HAL_SERVICE_ID_BLUETOOTH, HAL_SERVICE_ID_PAN);
> 
> +	test_opcode_valid("HANDSFREE", HAL_SERVICE_ID_HANDSFREE, 0x0f, 0,
> +						HAL_SERVICE_ID_BLUETOOTH,
> +						HAL_SERVICE_ID_HANDSFREE);
> +
>  	test_opcode_valid("A2DP", HAL_SERVICE_ID_A2DP, 0x03, 0,
>  			HAL_SERVICE_ID_BLUETOOTH, HAL_SERVICE_ID_A2DP);
> 
> +	test_opcode_valid("HEALTH", HAL_SERVICE_ID_HEALTH, 0x06, 0,
> +						HAL_SERVICE_ID_BLUETOOTH,
> +						HAL_SERVICE_ID_HEALTH);
> +
> +	test_opcode_valid("AVRCP", HAL_SERVICE_ID_AVRCP, 0x0b, 0,
> +				HAL_SERVICE_ID_BLUETOOTH, HAL_SERVICE_ID_AVRCP);
> +
> +	test_opcode_valid("GATT", HAL_SERVICE_ID_GATT, 0x24, 0,
> +				HAL_SERVICE_ID_BLUETOOTH, HAL_SERVICE_ID_GATT);
> +
>  	test_opcode_valid("HF_CLIENT", HAL_SERVICE_ID_HANDSFREE_CLIENT, 0x10, 0,
>  			HAL_SERVICE_ID_BLUETOOTH,
>  			HAL_SERVICE_ID_HANDSFREE_CLIENT);

In future please don't send unrelated patches as part of serie. This makes 
review easier and also makes no unnecessary delays in getting such patches 
merged.

Applied. Thanks.

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCH 03/10] android/map-client: Add support for get remote MAS instances
From: Szymon Janc @ 2014-10-13 20:19 UTC (permalink / raw)
  To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth
In-Reply-To: <1412858714-2845-4-git-send-email-grzegorz.kolodziejczyk@tieto.com>

On Thursday 09 October 2014 14:45:07 Grzegorz Kolodziejczyk wrote:
> This allows to get remote mas instances. In case of wrong sdp record
> fail status will be returned in notification.
> ---
>  android/map-client.c | 124
> ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 123
> insertions(+), 1 deletion(-)
> 
> diff --git a/android/map-client.c b/android/map-client.c
> index 1001b36..adeae4b 100644
> --- a/android/map-client.c
> +++ b/android/map-client.c
> @@ -30,21 +30,143 @@
>  #include <stdint.h>
>  #include <glib.h>
> 
> +#include "lib/sdp.h"
> +#include "lib/sdp_lib.h"
> +#include "src/sdp-client.h"
> +
>  #include "ipc.h"
>  #include "lib/bluetooth.h"
>  #include "map-client.h"
>  #include "src/log.h"
>  #include "hal-msg.h"
> +#include "ipc-common.h"
> +#include "utils.h"
> +#include "src/shared/util.h"
> 
>  static struct ipc *hal_ipc = NULL;
>  static bdaddr_t adapter_addr;
> 
> +static int fill_mce_inst(void *buf, int32_t id, int32_t scn, int32_t
> msg_type, +					const void *name, uint8_t name_len)
> +{
> +	struct hal_map_client_mas_instance *inst = buf;
> +
> +	inst->id = id;
> +	inst->scn = scn;
> +	inst->msg_types = msg_type;
> +	inst->name_len = name_len;
> +
> +	if (name_len)
> +		memcpy(inst->name, name, name_len);
> +
> +	return sizeof(*inst) + name_len;
> +}
> +
> +static void map_client_sdp_search_cb(sdp_list_t *recs, int err, gpointer
> data) +{
> +	uint8_t buf[IPC_MTU];
> +	struct hal_ev_map_client_remote_mas_instances *ev = (void *) buf;
> +	bdaddr_t *dst = data;
> +	sdp_list_t *list, *protos;
> +	uint8_t status;
> +	int32_t id, scn, msg_type, name_len, num_instances = 0;
> +	char *name;
> +	size_t size;
> +
> +	size = sizeof(*ev);
> +	bdaddr2android(dst, &ev->bdaddr);
> +
> +	if (err < 0) {
> +		error("mce: Unable to get SDP record: %s", strerror(-err));
> +		status = HAL_STATUS_FAILED;
> +		goto fail;
> +	}
> +
> +	for (list = recs; list != NULL; list = list->next) {
> +		sdp_record_t *rec = list->data;
> +		sdp_data_t *data;
> +
> +		data = sdp_data_get(rec, SDP_ATTR_MAS_INSTANCE_ID);
> +		if (data) {
> +			id = data->val.uint8;
> +		} else {
> +			error("mce: cannot get mas instance id");
> +			status = HAL_STATUS_FAILED;
> +			goto fail;

I'm not sure if we should fail here. Lets just skip record (we can leave error 
message) and continue with search.

Also make it like:  if (!data) {error(); continue;};
Not need for if-else

> +		}
> +
> +		data = sdp_data_get(rec, SDP_ATTR_SVCNAME_PRIMARY);
> +		if (data) {
> +			name = data->val.str;
> +			name_len = data->unitSize;
> +		} else {
> +			error("mce: cannot get mas instance name");
> +			status = HAL_STATUS_FAILED;
> +			goto fail;
> +		}
> +
> +		data = sdp_data_get(rec, SDP_ATTR_SUPPORTED_MESSAGE_TYPES);
> +		if (data) {
> +			msg_type = data->val.uint8;
> +		} else {
> +			error("mce: cannot get mas instance msg type");
> +			status = HAL_STATUS_FAILED;
> +			goto fail;
> +		}
> +
> +		if (!sdp_get_access_protos(rec, &protos)) {
> +			scn = sdp_get_proto_port(protos, RFCOMM_UUID);
> +
> +			sdp_list_foreach(protos,
> +					(sdp_list_func_t) sdp_list_free, NULL);
> +			sdp_list_free(protos, NULL);
> +		} else {
> +			error("mce: cannot get mas instance rfcomm channel");
> +			status = HAL_STATUS_FAILED;
> +			goto fail;
> +		}
> +
> +		size += fill_mce_inst(buf + size, id, scn, msg_type, name,
> +								name_len);
> +		num_instances++;
> +	}
> +
> +	status = HAL_STATUS_SUCCESS;

Please check if we are expected to return error if no instances were found.

> +
> +fail:
> +	ev->num_instances = num_instances;
> +	ev->status = status;
> +
> +	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT,
> +			HAL_EV_MAP_CLIENT_REMOTE_MAS_INSTANCES, size, buf);
> +
> +	free(dst);
> +}
> +
>  static void handle_get_instances(const void *buf, uint16_t len)
>  {
> +	const struct hal_cmd_map_client_get_instances *cmd = buf;
> +	uint8_t status;
> +	bdaddr_t *dst;
> +	uuid_t uuid;
> +
>  	DBG("");
> 
> +	dst = new0(bdaddr_t, 1);

Please check if allocation failed.

> +	android2bdaddr(&cmd->bdaddr, dst);
> +
> +	sdp_uuid16_create(&uuid, MAP_MSE_SVCLASS_ID);
> +
> +	if (bt_search_service(&adapter_addr, dst, &uuid,
> +				map_client_sdp_search_cb, dst, NULL, 0)) {

Just a hint: you can pass free as destroy function here instead of taking care 
of that in callback.

> +		error("mce: Failed to search SDP details");
> +		status = HAL_STATUS_FAILED;
> +		free(dst);
> +	}
> +
> +	status = HAL_STATUS_SUCCESS;

In case of error status is overwritten.

>  	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT,
> -			HAL_OP_MAP_CLIENT_GET_INSTANCES, HAL_STATUS_FAILED);
> +				HAL_OP_MAP_CLIENT_GET_INSTANCES, status);
>  }
> 
>  static const struct ipc_handler cmd_handlers[] = {

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCH 02/10] android/map-client: Add stubs for MAP client commands handlers
From: Szymon Janc @ 2014-10-13 19:58 UTC (permalink / raw)
  To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth
In-Reply-To: <1412858714-2845-3-git-send-email-grzegorz.kolodziejczyk@tieto.com>

Hi Grzegorz,

On Thursday 09 October 2014 14:45:06 Grzegorz Kolodziejczyk wrote:
> Add empty handlers for MAP client IPC commands.
> ---
>  android/map-client.c | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/android/map-client.c b/android/map-client.c
> index 4556461..1001b36 100644
> --- a/android/map-client.c
> +++ b/android/map-client.c
> @@ -28,17 +28,48 @@
>  #include <stdbool.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> +#include <glib.h>
> 
>  #include "ipc.h"
>  #include "lib/bluetooth.h"
>  #include "map-client.h"
> +#include "src/log.h"
> +#include "hal-msg.h"
> +
> +static struct ipc *hal_ipc = NULL;
> +static bdaddr_t adapter_addr;
> +
> +static void handle_get_instances(const void *buf, uint16_t len)
> +{
> +	DBG("");
> +
> +	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT,
> +			HAL_OP_MAP_CLIENT_GET_INSTANCES, HAL_STATUS_FAILED);
> +}
> +
> +static const struct ipc_handler cmd_handlers[] = {
> +	{handle_get_instances, false,
> +			sizeof(struct hal_cmd_map_client_get_instances)},

Style issue: there should be spaces after { and before }.
Also please add comment about define type just like in other handlers (I'm 
aware that there is just 1 handler here but lets be consistent).

> +};
> 
>  bool bt_map_client_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t
> mode) {
> -	return false;
> +	DBG("");
> +
> +	bacpy(&adapter_addr, addr);
> +
> +	hal_ipc = ipc;
> +
> +	ipc_register(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT, cmd_handlers,
> +						G_N_ELEMENTS(cmd_handlers));
> +
> +	return true;
>  }
> 
>  void bt_map_client_unregister(void)
>  {
> +	DBG("");
> 
> +	ipc_unregister(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT);
> +	hal_ipc = NULL;
>  }

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCH 1/2] android/pts: Update HSP files for PTS 5.3
From: Szymon Janc @ 2014-10-13 18:55 UTC (permalink / raw)
  To: Sebastian Chlad; +Cc: linux-bluetooth
In-Reply-To: <1412943017-17823-1-git-send-email-sebastian.chlad@tieto.com>

Hi Sebastian,

On Friday 10 October 2014 14:10:16 Sebastian Chlad wrote:
> ---
>  android/pics-hsp.txt  | 2 +-
>  android/pixit-hsp.txt | 2 +-
>  android/pts-hsp.txt   | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/android/pics-hsp.txt b/android/pics-hsp.txt
> index 330a197..7e71b14 100644
> --- a/android/pics-hsp.txt
> +++ b/android/pics-hsp.txt
> @@ -1,6 +1,6 @@
>  HSP PICS for the PTS tool.
> 
> -PTS version: 5.2
> +PTS version: 5.3
> 
>  * - different than PTS defaults
>  # - not yet implemented/supported
> diff --git a/android/pixit-hsp.txt b/android/pixit-hsp.txt
> index a097277..6be4731 100644
> --- a/android/pixit-hsp.txt
> +++ b/android/pixit-hsp.txt
> @@ -1,6 +1,6 @@
>  HSP PIXIT for the PTS tool.
> 
> -PTS version: 5.2
> +PTS version: 5.3
> 
>  * - different than PTS defaults
>  & - should be set to IUT Bluetooth address
> diff --git a/android/pts-hsp.txt b/android/pts-hsp.txt
> index 0df9806..9031b82 100644
> --- a/android/pts-hsp.txt
> +++ b/android/pts-hsp.txt
> @@ -1,7 +1,7 @@
>  PTS test results for HSP
> 
> -PTS version: 5.2
> -Tested: 15-July-2014
> +PTS version: 5.3
> +Tested: 10-October-2014
>  Android version: 4.4.4
> 
>  Results:

Both patches applied. Thanks.

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* [PATCH bluetooth] Bluetooth: Fix missing channel unlock in l2cap_le_credits
From: Martin Townsend @ 2014-10-13 18:24 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, jukka.rissanen, johan.hedberg, Martin Townsend

In the error case where credits is greater than max_credits there
is a missing l2cap_chan_unlock before returning.

Signed-off-by: Martin Townsend <mtownsend1973@gmail.com>
---
 net/bluetooth/l2cap_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 46547b9..bfb6af8 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5544,6 +5544,7 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn,
 	if (credits > max_credits) {
 		BT_ERR("LE credits overflow");
 		l2cap_send_disconn_req(chan, ECONNRESET);
+		l2cap_chan_unlock(chan);
 
 		/* Return 0 so that we don't trigger an unnecessary
 		 * command reject packet.
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression.
From: Alexander Aring @ 2014-10-13 17:22 UTC (permalink / raw)
  To: Jukka Rissanen
  Cc: Martin Townsend, Martin Townsend, linux-bluetooth, linux-wpan,
	marcel
In-Reply-To: <20141013171111.GA25249@omega>

Hi Jukka,

sorry.

I was a little too fast here, because I am sure now this should solve your
lockdep issue.

On Mon, Oct 13, 2014 at 07:11:11PM +0200, Alexander Aring wrote:
> Hi Jukka,
> 
> On Mon, Oct 13, 2014 at 06:09:14PM +0300, Jukka Rissanen wrote:
> > Hi Martin,
> > 
> > On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote:
> > > Hi Jukka,
> > > 
> > > Does this patch help?
> > 
> > Unfortunately no, I still see inconsistent lock state. It would probably
> > have been too easy :)
> > 
> 
> I remeber something, I think 802.15.4 had similar issues long time ago.
> 
s/remeber/remember/

> The fix was 20e7c4e80dcd01dad5e6c8b32455228b8fe9c619 ("6lowpan: fix
> lockdep splats"). Please check that, you need something like that!
> 

Something like that:

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 4ebc806..02fd21a 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -642,7 +642,27 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev)
        return err < 0 ? NET_XMIT_DROP : err;
 }
 
+static struct lock_class_key bt_tx_busylock;
+static struct lock_class_key bt_netdev_xmit_lock_key;
+
+static void bt_set_lockdep_class_one(struct net_device *dev,
+                                    struct netdev_queue *txq,
+                                    void *_unused)
+{
+       lockdep_set_class(&txq->_xmit_lock,
+                         &bt_netdev_xmit_lock_key);
+}
+
+
+static int bt_dev_init(struct net_device *dev)
+{
+       netdev_for_each_tx_queue(dev, bt_set_lockdep_class_one, NULL);
+       dev->qdisc_tx_busylock = &bt_tx_busylock;
+       return 0;
+}
+
 static const struct net_device_ops netdev_ops = {
+       .ndo_init               = bt_dev_init,
        .ndo_start_xmit         = bt_xmit,
 };


- Alex

[0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=20e7c4e80dcd01dad5e6c8b32455228b8fe9c619

^ permalink raw reply related

* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression.
From: Alexander Aring @ 2014-10-13 17:11 UTC (permalink / raw)
  To: Jukka Rissanen
  Cc: Martin Townsend, Martin Townsend, linux-bluetooth, linux-wpan,
	marcel
In-Reply-To: <1413212954.2705.106.camel@jrissane-mobl.ger.corp.intel.com>

Hi Jukka,

On Mon, Oct 13, 2014 at 06:09:14PM +0300, Jukka Rissanen wrote:
> Hi Martin,
> 
> On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote:
> > Hi Jukka,
> > 
> > Does this patch help?
> 
> Unfortunately no, I still see inconsistent lock state. It would probably
> have been too easy :)
> 

I remeber something, I think 802.15.4 had similar issues long time ago.

The fix was 20e7c4e80dcd01dad5e6c8b32455228b8fe9c619 ("6lowpan: fix
lockdep splats"). Please check that, you need something like that!

- Alex

[0] 

^ permalink raw reply

* Re: [PATCH v3 1/2] core: Add Manufacturer Specific Data EIR field
From: Alfonso Acosta @ 2014-10-13 17:02 UTC (permalink / raw)
  To: Alfonso Acosta, BlueZ development
In-Reply-To: <20141013161319.GA27533@t440s.P-661HNU-F1>

Hi Johan and Marcel,

>> +             case EIR_MANUFACTURER_DATA:
>> +                     if (data_len < 2 || data_len > 2 + sizeof(eir->msd->data))
>> +                             break;
>> +                     eir->msd = g_malloc(sizeof(*eir->msd));
>> +                     eir->msd->company = get_le16(data);
>> +                     eir->msd->data_len = data_len - 2;
>> +                     memcpy(&eir->msd->data, data + 2, eir->msd->data_len);
>> +                     break;
>
> Wouldn't this lead to a memory leaks if a device (violating the spec. but
> still) had two or more manufacturer data entries in it's AD/EIR data?
> Taking example from how remote name entries are handled you should
> probably g_free(eir->msd) before allocating a new one.

Very good point. So, should I support multiple MSD fields or just the
first (last) one for now?

In all honesty, just like Johan, I didn't even know it was legal to
provide multiple ones.

Thanks,

-- 
Alfonso Acosta

Embedded Systems Engineer at Spotify
Birger Jarlsgatan 61, Stockholm, Sweden
http://www.spotify.com

^ permalink raw reply

* Re: [PATCH v3 1/2] core: Add Manufacturer Specific Data EIR field
From: Johan Hedberg @ 2014-10-13 16:30 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Alfonso Acosta, linux-bluetooth
In-Reply-To: <D4814A57-3BAC-43BC-8C39-447FADE29B79@holtmann.org>

Hi Marcel,

On Mon, Oct 13, 2014, Marcel Holtmann wrote:
> Hi Johan,
> 
> >> +		case EIR_MANUFACTURER_DATA:
> >> +			if (data_len < 2 || data_len > 2 + sizeof(eir->msd->data))
> >> +				break;
> >> +			eir->msd = g_malloc(sizeof(*eir->msd));
> >> +			eir->msd->company = get_le16(data);
> >> +			eir->msd->data_len = data_len - 2;
> >> +			memcpy(&eir->msd->data, data + 2, eir->msd->data_len);
> >> +			break;
> > 
> > Wouldn't this lead to a memory leaks if a device (violating the spec. but
> > still) had two or more manufacturer data entries in it's AD/EIR data?
> > Taking example from how remote name entries are handled you should
> > probably g_free(eir->msd) before allocating a new one.
> 
> have multiple manufacturer data entries is not violating the
> specification. That is actually valid.

Right you are. I was somehow assuming this would be similar to e.g. the
UUID fields. Anyway, if we want to represent in the new internal API all
that is allowed in the spec it'd need some redesign. OTOH, I'm not sure
it's worth it until we have an example of an actual product that has
multiple entries).

Johan

^ permalink raw reply

* Re: [PATCH v3 1/2] core: Add Manufacturer Specific Data EIR field
From: Marcel Holtmann @ 2014-10-13 16:18 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: Alfonso Acosta, linux-bluetooth
In-Reply-To: <20141013161319.GA27533@t440s.P-661HNU-F1>

Hi Johan,

>> +		case EIR_MANUFACTURER_DATA:
>> +			if (data_len < 2 || data_len > 2 + sizeof(eir->msd->data))
>> +				break;
>> +			eir->msd = g_malloc(sizeof(*eir->msd));
>> +			eir->msd->company = get_le16(data);
>> +			eir->msd->data_len = data_len - 2;
>> +			memcpy(&eir->msd->data, data + 2, eir->msd->data_len);
>> +			break;
> 
> Wouldn't this lead to a memory leaks if a device (violating the spec. but
> still) had two or more manufacturer data entries in it's AD/EIR data?
> Taking example from how remote name entries are handled you should
> probably g_free(eir->msd) before allocating a new one.

have multiple manufacturer data entries is not violating the specification. That is actually valid.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v3 1/2] core: Add Manufacturer Specific Data EIR field
From: Johan Hedberg @ 2014-10-13 16:13 UTC (permalink / raw)
  To: Alfonso Acosta; +Cc: linux-bluetooth
In-Reply-To: <1413200623-31278-3-git-send-email-fons@spotify.com>

Hi Alfonso,

On Mon, Oct 13, 2014, Alfonso Acosta wrote:
> +		case EIR_MANUFACTURER_DATA:
> +			if (data_len < 2 || data_len > 2 + sizeof(eir->msd->data))
> +				break;
> +			eir->msd = g_malloc(sizeof(*eir->msd));
> +			eir->msd->company = get_le16(data);
> +			eir->msd->data_len = data_len - 2;
> +			memcpy(&eir->msd->data, data + 2, eir->msd->data_len);
> +			break;

Wouldn't this lead to a memory leaks if a device (violating the spec. but
still) had two or more manufacturer data entries in it's AD/EIR data?
Taking example from how remote name entries are handled you should
probably g_free(eir->msd) before allocating a new one.

Johan

^ permalink raw reply

* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression.
From: Johan Hedberg @ 2014-10-13 16:07 UTC (permalink / raw)
  To: Jukka Rissanen
  Cc: Martin Townsend, Martin Townsend, linux-bluetooth, linux-wpan,
	marcel, alex.aring
In-Reply-To: <1413212954.2705.106.camel@jrissane-mobl.ger.corp.intel.com>

Hi,

On Mon, Oct 13, 2014, Jukka Rissanen wrote:
> On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote:
> > Hi Jukka,
> > 
> > Does this patch help?
> 
> Unfortunately no, I still see inconsistent lock state. It would probably
> have been too easy :)
> > 
> > ---
> >  net/bluetooth/l2cap_core.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index b6f9777..fb7b2ff 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -5494,6 +5494,7 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn,
> >  	if (credits > max_credits) {
> >  		BT_ERR("LE credits overflow");
> >  		l2cap_send_disconn_req(chan, ECONNRESET);
> > +		l2cap_chan_unlock(chan);
> >  
> >  		/* Return 0 so that we don't trigger an unnecessary
> >  		 * command reject packet.

I'd appreciate if you could still send a proper patch for this since
it's clearly a locking bug (something even worth sending to stable
trees).

Johan

^ permalink raw reply

* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression.
From: Martin Townsend @ 2014-10-13 16:00 UTC (permalink / raw)
  To: Jukka Rissanen
  Cc: Martin Townsend, linux-bluetooth, linux-wpan, marcel, alex.aring
In-Reply-To: <1413212954.2705.106.camel@jrissane-mobl.ger.corp.intel.com>

Hi Jukka,

>From the last trace it looks like a transmit has been started from the receive worker thread.  I notice that both tx and rx use the same workerqueue structure, e.g.

hci_send_xxx
...
queue_work(hdev->workqueue, &hdev->tx_work);


hci_recv_frame
...
queue_work(hdev->workqueue, &hdev->rx_work);


Would this cause problems.  I don't know enough about work queues but in workqueue_struct there is a mutex which may be held by the rx worker and if this rx worker start a transmit maybe this would cause the lock?  Could test by creating separate queues for rx and tx?

Anyway just a thought.

- Martin.


On 13/10/14 16:09, Jukka Rissanen wrote:
> Hi Martin,
>
> On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote:
>> Hi Jukka,
>>
>> Does this patch help?
> Unfortunately no, I still see inconsistent lock state. It would probably
> have been too easy :)
>
>> ---
>>  net/bluetooth/l2cap_core.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index b6f9777..fb7b2ff 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -5494,6 +5494,7 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn,
>>  	if (credits > max_credits) {
>>  		BT_ERR("LE credits overflow");
>>  		l2cap_send_disconn_req(chan, ECONNRESET);
>> +		l2cap_chan_unlock(chan);
>>  
>>  		/* Return 0 so that we don't trigger an unnecessary
>>  		 * command reject packet.
>
> Cheers,
> Jukka
>
>


^ permalink raw reply

* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression.
From: Martin Townsend @ 2014-10-13 15:47 UTC (permalink / raw)
  To: Jukka Rissanen
  Cc: Martin Townsend, linux-bluetooth, linux-wpan, marcel, alex.aring
In-Reply-To: <1413212954.2705.106.camel@jrissane-mobl.ger.corp.intel.com>

[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]

Hi Jukka,

>From the last trace it looks like a transmit has been started from the receive worker thread.  I notice that both tx and rx use the same workerqueue structure, e.g.

hci_send_xxx <http://lxr.free-electrons.com/ident?i=hci_send_acl>
...
queue_work <http://lxr.free-electrons.com/ident?i=queue_work>(hdev <http://lxr.free-electrons.com/ident?i=hdev>->workqueue <http://lxr.free-electrons.com/ident?i=workqueue>, &hdev <http://lxr.free-electrons.com/ident?i=hdev>->tx_work);



hci_recv_frame <http://lxr.free-electrons.com/ident?i=hci_recv_frame>
...
queue_work <http://lxr.free-electrons.com/ident?i=queue_work>(hdev <http://lxr.free-electrons.com/ident?i=hdev>->workqueue <http://lxr.free-electrons.com/ident?i=workqueue>, &hdev <http://lxr.free-electrons.com/ident?i=hdev>->rx_work <http://lxr.free-electrons.com/ident?i=rx_work>);


Would this cause problems.  I don't know enough about work queues but in workqueue_struct there is a mutex which may be held by the rx worker and if this rx worker start a transmit maybe this would cause the lock?  Could test by creating separate queues for rx and tx?

Anyway just a thought.

- Martin.


On 13/10/14 16:09, Jukka Rissanen wrote:
> Hi Martin,
>
> On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote:
>> Hi Jukka,
>>
>> Does this patch help?
> Unfortunately no, I still see inconsistent lock state. It would probably
> have been too easy :)
>
>> ---
>>  net/bluetooth/l2cap_core.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index b6f9777..fb7b2ff 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -5494,6 +5494,7 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn,
>>  	if (credits > max_credits) {
>>  		BT_ERR("LE credits overflow");
>>  		l2cap_send_disconn_req(chan, ECONNRESET);
>> +		l2cap_chan_unlock(chan);
>>  
>>  		/* Return 0 so that we don't trigger an unnecessary
>>  		 * command reject packet.
>
> Cheers,
> Jukka
>
>


[-- Attachment #2: Type: text/html, Size: 2931 bytes --]

^ permalink raw reply

* Re: [PATCH v6 bluetooth-next] 6lowpan: Use skb_cow in IPHC decompression.
From: Jukka Rissanen @ 2014-10-13 15:09 UTC (permalink / raw)
  To: Martin Townsend
  Cc: Martin Townsend, linux-bluetooth, linux-wpan, marcel, alex.aring
In-Reply-To: <543BE816.80304@xsilon.com>

Hi Martin,

On ma, 2014-10-13 at 15:56 +0100, Martin Townsend wrote:
> Hi Jukka,
> 
> Does this patch help?

Unfortunately no, I still see inconsistent lock state. It would probably
have been too easy :)

> 
> ---
>  net/bluetooth/l2cap_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index b6f9777..fb7b2ff 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -5494,6 +5494,7 @@ static inline int l2cap_le_credits(struct l2cap_conn *conn,
>  	if (credits > max_credits) {
>  		BT_ERR("LE credits overflow");
>  		l2cap_send_disconn_req(chan, ECONNRESET);
> +		l2cap_chan_unlock(chan);
>  
>  		/* Return 0 so that we don't trigger an unnecessary
>  		 * command reject packet.


Cheers,
Jukka

^ permalink raw reply


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