Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH 6/6] avctp: Fix wrong error message
From: Andrei Emeltchenko @ 2014-01-28 13:22 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1390915365-4335-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 profiles/audio/avctp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 0a1a457..9f87859 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -933,7 +933,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
 
 	ret -= AVCTP_HEADER_LENGTH;
 	if (ret < AVC_HEADER_LENGTH) {
-		error("Too small AVCTP packet");
+		error("Too small AVC packet");
 		goto failed;
 	}
 
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 5/6] avctp: Use predefined HEADER_LENGTH instead of sizeof
From: Andrei Emeltchenko @ 2014-01-28 13:22 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1390915365-4335-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Make code consistent with using HEADER_LENGTH already defined and used
instead of sizeof(). Remove also ugly type conversion.
---
 profiles/audio/avctp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 570d609..0a1a457 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -924,24 +924,24 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
 	if (ret <= 0)
 		goto failed;
 
-	if ((unsigned int) ret < sizeof(struct avctp_header)) {
+	if (ret < AVCTP_HEADER_LENGTH) {
 		error("Too small AVCTP packet");
 		goto failed;
 	}
 
 	avctp = (struct avctp_header *) buf;
 
-	ret -= sizeof(struct avctp_header);
-	if ((unsigned int) ret < sizeof(struct avc_header)) {
+	ret -= AVCTP_HEADER_LENGTH;
+	if (ret < AVC_HEADER_LENGTH) {
 		error("Too small AVCTP packet");
 		goto failed;
 	}
 
-	avc = (struct avc_header *) (buf + sizeof(struct avctp_header));
+	avc = (struct avc_header *) (buf + AVCTP_HEADER_LENGTH);
 
-	ret -= sizeof(struct avc_header);
+	ret -= AVC_HEADER_LENGTH;
 
-	operands = (uint8_t *) avc + sizeof(struct avc_header);
+	operands = (uint8_t *) avc + AVC_HEADER_LENGTH;
 	operand_count = ret;
 
 	if (avctp->cr == AVCTP_RESPONSE) {
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 4/6] avctp: trivial: Do not break short line
From: Andrei Emeltchenko @ 2014-01-28 13:22 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1390915365-4335-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 profiles/audio/avctp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 78bf1fd..570d609 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -904,8 +904,7 @@ failed:
 	return FALSE;
 }
 
-static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
-				gpointer data)
+static gboolean session_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
 {
 	struct avctp *session = data;
 	struct avctp_channel *control = session->control;
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 3/6] avctp: Use already calculated avc header
From: Andrei Emeltchenko @ 2014-01-28 13:22 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1390915365-4335-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

This removes long (81 characters) line as a side effect.
---
 profiles/audio/avctp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 6fd1454..78bf1fd 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -942,7 +942,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
 
 	ret -= sizeof(struct avc_header);
 
-	operands = buf + sizeof(struct avctp_header) + sizeof(struct avc_header);
+	operands = (uint8_t *) avc + sizeof(struct avc_header);
 	operand_count = ret;
 
 	if (avctp->cr == AVCTP_RESPONSE) {
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 2/6] android/avctp: Fix wrong error message
From: Andrei Emeltchenko @ 2014-01-28 13:22 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1390915365-4335-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/avctp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/avctp.c b/android/avctp.c
index 4ebf2ff..d308ec1 100644
--- a/android/avctp.c
+++ b/android/avctp.c
@@ -827,7 +827,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
 
 	ret -= AVCTP_HEADER_LENGTH;
 	if (ret < AVC_HEADER_LENGTH) {
-		error("Too small AVCTP packet");
+		error("Too small AVC packet");
 		goto failed;
 	}
 
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 1/6] android/avctp: Use predefined HEADER_LENGTH instead of sizeof
From: Andrei Emeltchenko @ 2014-01-28 13:22 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1390915365-4335-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Make code consistent with using HEADER_LENGTH defined. Remove also type
conversion.
---
 android/avctp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/android/avctp.c b/android/avctp.c
index 6f9b33b..4ebf2ff 100644
--- a/android/avctp.c
+++ b/android/avctp.c
@@ -818,24 +818,24 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
 	if (ret <= 0)
 		goto failed;
 
-	if ((unsigned int) ret < sizeof(struct avctp_header)) {
+	if (ret < AVCTP_HEADER_LENGTH) {
 		error("Too small AVCTP packet");
 		goto failed;
 	}
 
 	avctp = (struct avctp_header *) buf;
 
-	ret -= sizeof(struct avctp_header);
-	if ((unsigned int) ret < sizeof(struct avc_header)) {
+	ret -= AVCTP_HEADER_LENGTH;
+	if (ret < AVC_HEADER_LENGTH) {
 		error("Too small AVCTP packet");
 		goto failed;
 	}
 
-	avc = (struct avc_header *) (buf + sizeof(struct avctp_header));
+	avc = (struct avc_header *) (buf + AVCTP_HEADER_LENGTH);
 
-	ret -= sizeof(struct avc_header);
+	ret -= AVC_HEADER_LENGTH;
 
-	operands = (uint8_t *) avc + sizeof(struct avc_header);
+	operands = (uint8_t *) avc + AVC_HEADER_LENGTH;
 	operand_count = ret;
 
 	if (avctp->cr == AVCTP_RESPONSE) {
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 0/6] Small set of AVCTP fixes
From: Andrei Emeltchenko @ 2014-01-28 13:22 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

AVCTP issues found during writing test cases.

Andrei Emeltchenko (6):
  android/avctp: Use predefined HEADER_LENGTH instead of sizeof
  android/avctp: Fix wrong error message
  avctp: Use already calculated avc header
  avctp: trivial: Do not break short line
  avctp: Use predefined HEADER_LENGTH instead of sizeof
  avctp: Fix wrong error message

 android/avctp.c        | 14 +++++++-------
 profiles/audio/avctp.c | 17 ++++++++---------
 2 files changed, 15 insertions(+), 16 deletions(-)

-- 
1.8.3.2


^ permalink raw reply

* [PATCH] android/pts: PTS test results for GAP
From: Sebastian Chlad @ 2014-01-28 12:56 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sebastian Chlad

---
 android/pts-gap.txt | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/android/pts-gap.txt b/android/pts-gap.txt
index 0ea5539..1c0feec 100644
--- a/android/pts-gap.txt
+++ b/android/pts-gap.txt
@@ -1,7 +1,7 @@
 PTS test results for GAP
 
-PTS version: 4.9
-Tested: 14.11.2013
+PTS version: 5.0
+Tested: 28.01.2014
 
 Results:
 PASS	test passed
@@ -12,14 +12,14 @@ N/A	test is disabled due to PICS setup
 -------------------------------------------------------------------------------
 Test Name		Result	Notes
 -------------------------------------------------------------------------------
-TC_MOD_NDIS_BV_01_C	PASS
+TC_MOD_NDIS_BV_01_C	PASS	IUT must be in non-discoverable mode
 TC_MOD_LDIS_BV_01_C	N/A
 TC_MOD_LDIS_BV_02_C	N/A
 TC_MOD_LDIS_BV_03_C	N/A
-TC_MOD_GDIS_BV_01_C	PASS
-TC_MOD_GDIS_BV_02_C	PASS
-TC_MOD_NCON_BV_01_C	PASS
-TC_MOD_CON_BV_01_C	PASS
+TC_MOD_GDIS_BV_01_C	PASS	IUT must be discoverable
+TC_MOD_GDIS_BV_02_C	PASS	IUT must be discoverable
+TC_MOD_NCON_BV_01_C	PASS	btmgmt connectable off
+TC_MOD_CON_BV_01_C	PASS	btmgmt connectable on
 TC_BROB_BCST_BV_01_C	N/A
 TC_BROB_BCST_BV_02_C	N/A
 TC_BROB_BCST_BV_03_C	N/A
@@ -48,7 +48,7 @@ TC_DISC_GENP_BV_02_C	INC	LE not supported yet
 TC_DISC_GENP_BV_03_C	INC	LE not supported yet
 TC_DISC_GENP_BV_04_C	INC	LE not supported yet
 TC_DISC_GENP_BV_05_C	INC	LE not supported yet
-TC_IDLE_GIN_BV_01_C	PASS
+TC_IDLE_GIN_BV_01_C	PASS	Start inquiry from IUT
 TC_IDLE_LIN_BV_01_C	N/A
 TC_IDLE_NAMP_BV_01_C	INC	LE not supported yet
 TC_IDLE_NAMP_BV_02_C	INC	LE not supported yet
@@ -144,7 +144,7 @@ TC_DM_NBON_BV_01_C	N/A
 TC_DM_BON_BV_01_C	INC	LE not supported yet
 TC_DM_GIN_BV_01_C	INC	LE not supported yet
 TC_DM_LIN_BV_01_C	N/A
-TC_DM_NAD_BV_01_C	PASS
+TC_DM_NAD_BV_01_C	PASS	Start inquiry from IUT
 TC_DM_NAD_BV_02_C	INC	LE not supported yet
 TC_DM_LEP_BV_01_C	N/A
 TC_DM_LEP_BV_02_C	N/A
-- 
1.8.3.2

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related

* [RFC] android/avctp: Move struct definitions to header
From: Andrei Emeltchenko @ 2014-01-28 12:34 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

There is currently inconsistence in the avctp code with
AVC_HEADER_LENGTH defined in avctp.h but AVCTP_HEADER_LENGTH defined in
avctp.c. The patch moves structure definitions to the header in
consistent way.
---
 android/avctp.c | 42 ------------------------------------------
 android/avctp.h | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 43 deletions(-)

diff --git a/android/avctp.c b/android/avctp.c
index af2046a..7aa0673 100644
--- a/android/avctp.c
+++ b/android/avctp.c
@@ -66,48 +66,6 @@
 #define AVCTP_PACKET_CONTINUE	2
 #define AVCTP_PACKET_END	3
 
-#if __BYTE_ORDER == __LITTLE_ENDIAN
-
-struct avctp_header {
-	uint8_t ipid:1;
-	uint8_t cr:1;
-	uint8_t packet_type:2;
-	uint8_t transaction:4;
-	uint16_t pid;
-} __attribute__ ((packed));
-#define AVCTP_HEADER_LENGTH 3
-
-struct avc_header {
-	uint8_t code:4;
-	uint8_t _hdr0:4;
-	uint8_t subunit_id:3;
-	uint8_t subunit_type:5;
-	uint8_t opcode;
-} __attribute__ ((packed));
-
-#elif __BYTE_ORDER == __BIG_ENDIAN
-
-struct avctp_header {
-	uint8_t transaction:4;
-	uint8_t packet_type:2;
-	uint8_t cr:1;
-	uint8_t ipid:1;
-	uint16_t pid;
-} __attribute__ ((packed));
-#define AVCTP_HEADER_LENGTH 3
-
-struct avc_header {
-	uint8_t _hdr0:4;
-	uint8_t code:4;
-	uint8_t subunit_type:5;
-	uint8_t subunit_id:3;
-	uint8_t opcode;
-} __attribute__ ((packed));
-
-#else
-#error "Unknown byte order"
-#endif
-
 struct avctp_control_req {
 	struct avctp_pending_req *p;
 	uint8_t code;
diff --git a/android/avctp.h b/android/avctp.h
index a22bf13..428eeb8 100644
--- a/android/avctp.h
+++ b/android/avctp.h
@@ -26,7 +26,6 @@
 #define AVCTP_BROWSING_PSM		27
 
 #define AVC_MTU 512
-#define AVC_HEADER_LENGTH 3
 
 /* ctype entries */
 #define AVC_CTYPE_CONTROL		0x0
@@ -80,6 +79,50 @@
 #define AVC_F3				0x73
 #define AVC_F4				0x74
 
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+
+struct avctp_header {
+	uint8_t ipid:1;
+	uint8_t cr:1;
+	uint8_t packet_type:2;
+	uint8_t transaction:4;
+	uint16_t pid;
+} __attribute__ ((packed));
+#define AVCTP_HEADER_LENGTH 3
+
+struct avc_header {
+	uint8_t code:4;
+	uint8_t _hdr0:4;
+	uint8_t subunit_id:3;
+	uint8_t subunit_type:5;
+	uint8_t opcode;
+} __attribute__ ((packed));
+#define AVC_HEADER_LENGTH 3
+
+#elif __BYTE_ORDER == __BIG_ENDIAN
+
+struct avctp_header {
+	uint8_t transaction:4;
+	uint8_t packet_type:2;
+	uint8_t cr:1;
+	uint8_t ipid:1;
+	uint16_t pid;
+} __attribute__ ((packed));
+#define AVCTP_HEADER_LENGTH 3
+
+struct avc_header {
+	uint8_t _hdr0:4;
+	uint8_t code:4;
+	uint8_t subunit_type:5;
+	uint8_t subunit_id:3;
+	uint8_t opcode;
+} __attribute__ ((packed));
+#define AVC_HEADER_LENGTH 3
+
+#else
+#error "Unknown byte order"
+#endif
+
 struct avctp;
 
 typedef bool (*avctp_passthrough_cb) (struct avctp *session,
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH] android/pts: Add PTS test results for AVRCP
From: Sebastian Chlad @ 2014-01-28 12:25 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sebastian Chlad

---
 android/pts-avrcp.txt | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/android/pts-avrcp.txt b/android/pts-avrcp.txt
index f2533b4..6ad97f7 100644
--- a/android/pts-avrcp.txt
+++ b/android/pts-avrcp.txt
@@ -1,7 +1,7 @@
 PTS test results for AVRCP
 
 PTS version: 5.0
-Tested: --not yet tested--
+Tested: 28.01.2014
 
 Results:
 PASS	test passed
@@ -91,17 +91,17 @@ Test Name		Result	Notes
 -------------------------------------------------------------------------------
 TC_TG_BGN_BV_01_I	N/A
 TC_TG_BGN_BV_02_I	N/A
-TC_TG_CEC_BV_01_I	INC
-TC_TG_CEC_BV_02_I	INC
+TC_TG_CEC_BV_01_I	PASS
+TC_TG_CEC_BV_02_I	INC	Initiate a control chanenel connection
 TC_TG_CFG_BI_01_C	INC
 TC_TG_CFG_BV_02_C	INC
 TC_TG_CON_BV_02_C	N/A
 TC_TG_CON_BV_04_C	N/A
 TC_TG_CON_BV_05_C	N/A
-TC_TG_CRC_BV_01_I	INC
-TC_TG_CRC_BV_02_I	INC
-TC_TG_ICC_BV_01_I	INC
-TC_TG_ICC_BV_02_I	INC
+TC_TG_CRC_BV_01_I	PASS
+TC_TG_CRC_BV_02_I	PASS	Disconnect from PTS
+TC_TG_ICC_BV_01_I	PASS
+TC_TG_ICC_BV_02_I	PASS
 TC_TG_INV_BI_01_C	INC
 TC_TG_INV_BI_02_C	N/A
 TC_TG_MCN_CB_BI_01_C	N/A
-- 
1.8.3.2

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related

* [PATCH] android/pts: PTS test results for A2DP
From: Sebastian Chlad @ 2014-01-28 12:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sebastian Chlad

---
 android/pts-a2dp.txt | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/android/pts-a2dp.txt b/android/pts-a2dp.txt
index c6eee62..67f9183 100644
--- a/android/pts-a2dp.txt
+++ b/android/pts-a2dp.txt
@@ -1,7 +1,7 @@
 PTS test results for A2DP
 
 PTS version: 5.0
-Tested: 24.01.2014
+Tested: 28.01.2014
 
 Results:
 PASS	test passed
@@ -15,18 +15,20 @@ Test Name		Result	Notes
 -------------------------------------------------------------------------------
 TC_SRC_CC_BV_09_I	INC
 TC_SRC_CC_BV_10_I	N/A
-TC_SRC_REL_BV_01_I	PASS
+TC_SRC_REL_BV_01_I	PASS	Connect to PTS from IUT. When requested
+					disconnect from IUT
 TC_SRC_REL_BV_02_I	PASS
-TC_SRC_SET_BV_01_I	PASS
+TC_SRC_SET_BV_01_I	PASS	Connect to PTS (open a2dp)
 TC_SRC_SET_BV_02_I	PASS
-TC_SRC_SET_BV_03_I	PASS
-TC_SRC_SET_BV_04_I	INC
-TC_SRC_SET_BV_05_I	INC
-TC_SRC_SET_BV_06_I	INC
-TC_SRC_SUS_BV_01_I	INC
+TC_SRC_SET_BV_03_I	PASS	Start streaming
+TC_SRC_SET_BV_04_I	INC	Start streaming
+TC_SRC_SET_BV_05_I	INC	IUT must be moved out of range
+TC_SRC_SET_BV_06_I	INC	IUT must be moved out of range
+TC_SRC_SUS_BV_01_I	PASS	Stop streaming
 TC_SRC_SUS_BV_02_I	INC
 TC_SRC_SDP_BV_01_I	PASS
-TC_SRC_AS_BV_01_I	INC
+TC_SRC_AS_BV_01_I	PASS	Requires checking if the output on the IUT is
+					correct
 -------------------------------------------------------------------------------
 
 
-- 
1.8.3.2

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related

* Re: [RFC] Initial avctp test
From: Marcel Holtmann @ 2014-01-28 12:10 UTC (permalink / raw)
  To: Andrei Emeltchenko
  Cc: Luiz Augusto von Dentz, linux-bluetooth@vger.kernel.org
In-Reply-To: <20140128103641.GB9043@aemeltch-MOBL1>

Hi Andrei,

>>> This is initial invalid packet size test, sent as RFC since I am not
>>> sure what might be tested for avctp.
>>> 
>>> Andrei Emeltchenko (1):
>>>  unit/avctp: Add initial AVCTP test setup
>>> 
>>> Makefile.am       |   8 ++
>>> unit/test-avctp.c | 262 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 270 insertions(+)
>>> create mode 100644 unit/test-avctp.c
>>> 
>>> --
>>> 1.8.3.2
>> 
>> You should follow the testing spec:
>> https://www.bluetooth.org/docman/handlers/DownloadDoc.ashx?doc_id=40404
>> 
>> Please follow the same name convention that is used by the testing
>> spec and for now do not introduce any other test that is not part of
>> the testing spec.
> 
> I have this AVCTP.TS spec but what might be tested for example with test
> case "TP/CCM/BV-01-C"?
> 
> Even looking at session_cb() for AVCTP there seems to be only packet
> length check. In contrast for AVDTP there is command parser so you might
> be able to test something.
> 
> Shall we test AVRCP instead?

AVRCP and AVCTP need to be tested. This is not one or the other. It is both.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v3 0/4] Regression fixes for rfcomm/tty.c
From: Marcel Holtmann @ 2014-01-28 12:08 UTC (permalink / raw)
  To: Gianluca Anzolin
  Cc: Alexander Holler, Gustavo F. Padovan, peter,
	linux-bluetooth@vger.kernel.org development, Greg KH, jslaby,
	stable
In-Reply-To: <20140128083154.GA29060@sottospazio.it>

Hi Gianluca,

>>>> all 4 patches have been applied to bluetooth-next tree.
>>> 
>>> Maybe a bit late, but I've just seen they miss a Cc: stable@vger.kernel.org to automatically end up in 3.12 and 3.13 too.
>> 
>> we can always promote them to stable. On Purpose I wanted them to cycle through bluetooth-next for a while to make sure they do not cause any other regressions.
>> 
> 
> Unfortunately it seems I overlooked the fact that rfcomm_dev_activate() is
> called with the port->mutex held. So patches 2/3/4 cause a regression I missed
> because I didn't turn on the appropriate debug options (circular locking
> dependency, a bug report already appeared on this list).
> 
> I'm afraid this all stems from my partial knowledge of the tty_port code and
> unfortunately I don't know how to solve the problem right now.
> 
> I think it's better to revert those patches for the moment.

can we get this fixed instead. If I revert them, I have a different issue. So I am trading one bug for another one.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH BlueZ v2 1/4] android/AVDTP: Make signalling channel priority 6
From: Marcel Holtmann @ 2014-01-28 12:05 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: BlueZ development
In-Reply-To: <1390889820-24709-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

> This makes signalling priority 6 so it can push commands before the
> stream channel, without this the stream channel may be schedule
> first and cause the signalling commands to timeout while waiting a slot.
> ---
> v2: Return error if writes fails since that probably means the socket has been
> disconnected, also makes code setting socket to blocking a bit cleaner.
> 
> android/avdtp.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
> 
> diff --git a/android/avdtp.c b/android/avdtp.c
> index 4abcd75..4cfffc8 100644
> --- a/android/avdtp.c
> +++ b/android/avdtp.c
> @@ -2057,6 +2057,7 @@ struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
> 	struct avdtp *session;
> 	GIOCondition cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> 	int new_fd;
> +	int priority;

make it “int new_fd, priority;”.

> 
> 	new_fd = dup(fd);
> 	if (new_fd < 0) {
> @@ -2064,6 +2065,14 @@ struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
> 		return NULL;
> 	}
> 
> +	priority = 6;
> +	if (setsockopt(new_fd, SOL_SOCKET, SO_PRIORITY,
> +			(const void *) &priority, sizeof(priority)) < 0) {

What is this (const void *) cast for. That should not be needed.

> +		error("setsockopt(SO_PRIORITY): %s (%d)", strerror(errno),
> +									errno);
> +		return NULL;
> +	}
> +
> 	session = g_new0(struct avdtp, 1);
> 	session->io = g_io_channel_unix_new(new_fd);
> 	session->version = version;

Regards

Marcel


^ permalink raw reply

* Re: [RFC] Initial avctp test
From: Andrei Emeltchenko @ 2014-01-28 10:36 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZKusSmbTTEVLmU2Xa4+L5dOaKCHt8dFaeT2jP_==uL+jw@mail.gmail.com>

Hi Luiz,

On Mon, Jan 27, 2014 at 10:36:42AM -0800, Luiz Augusto von Dentz wrote:
> Hi Andrei,
> 
> On Mon, Jan 27, 2014 at 7:56 AM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > This is initial invalid packet size test, sent as RFC since I am not
> > sure what might be tested for avctp.
> >
> > Andrei Emeltchenko (1):
> >   unit/avctp: Add initial AVCTP test setup
> >
> >  Makefile.am       |   8 ++
> >  unit/test-avctp.c | 262 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 270 insertions(+)
> >  create mode 100644 unit/test-avctp.c
> >
> > --
> > 1.8.3.2
> 
> You should follow the testing spec:
> https://www.bluetooth.org/docman/handlers/DownloadDoc.ashx?doc_id=40404
> 
> Please follow the same name convention that is used by the testing
> spec and for now do not introduce any other test that is not part of
> the testing spec.

I have this AVCTP.TS spec but what might be tested for example with test
case "TP/CCM/BV-01-C"?

Even looking at session_cb() for AVCTP there seems to be only packet
length check. In contrast for AVDTP there is command parser so you might
be able to test something.

Shall we test AVRCP instead?

Best regards 
Andrei Emeltchenko 

^ permalink raw reply

* Re: [PATCH v3 0/4] Regression fixes for rfcomm/tty.c
From: Gianluca Anzolin @ 2014-01-28  8:31 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Alexander Holler, Gustavo F. Padovan, peter,
	linux-bluetooth@vger.kernel.org development, Greg KH, jslaby,
	stable
In-Reply-To: <E0DDC796-EECF-46B6-8EDF-801CF8EFD456@holtmann.org>

On Mon, Jan 20, 2014 at 09:37:14AM -0800, Marcel Holtmann wrote:
> Hi Alexander,
> 
> >> all 4 patches have been applied to bluetooth-next tree.
> > 
> > Maybe a bit late, but I've just seen they miss a Cc: stable@vger.kernel.org to automatically end up in 3.12 and 3.13 too.
> 
> we can always promote them to stable. On Purpose I wanted them to cycle through bluetooth-next for a while to make sure they do not cause any other regressions.
> 
> Regards
> 
> Marcel
> 

Unfortunately it seems I overlooked the fact that rfcomm_dev_activate() is
called with the port->mutex held. So patches 2/3/4 cause a regression I missed
because I didn't turn on the appropriate debug options (circular locking
dependency, a bug report already appeared on this list).

I'm afraid this all stems from my partial knowledge of the tty_port code and
unfortunately I don't know how to solve the problem right now.

I think it's better to revert those patches for the moment.

Regards,

Gianluca

^ permalink raw reply

* [PATCH BlueZ v2 4/4] android/hal-audio: Set stream fd to blocking
From: Luiz Augusto von Dentz @ 2014-01-28  6:17 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1390889820-24709-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes the stream to block on io operation so it does not return
EAGAIN on syscall such as write.
---
 android/hal-audio.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 2ca6289..21c1d94 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -25,6 +25,7 @@
 #include <sys/un.h>
 #include <unistd.h>
 #include <arpa/inet.h>
+#include <fcntl.h>
 
 #include <hardware/audio.h>
 #include <hardware/hardware.h>
@@ -1121,7 +1122,7 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 	struct audio_preset *preset;
 	const struct audio_codec *codec;
 	uint16_t mtu;
-	int fd;
+	int fd, flags;
 
 	out = calloc(1, sizeof(struct a2dp_stream_out));
 	if (!out)
@@ -1156,8 +1157,18 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 	if (!preset || fd < 0)
 		goto fail;
 
-	out->ep->fd = fd;
+	flags = fcntl(fd, F_GETFL, 0);
+	if (flags < 0) {
+		error("fcntl(F_GETFL): %s (%d)", strerror(errno), errno);
+		goto fail;
+	}
 
+	if (fcntl(fd, F_SETFL, flags & ~O_NONBLOCK) < 0) {
+		error("fcntl(F_SETFL): %s (%d)", strerror(errno), errno);
+		goto fail;
+	}
+
+	out->ep->fd = fd;
 	codec = out->ep->codec;
 
 	codec->init(preset, mtu, &out->ep->codec_data);
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH BlueZ v2 3/4] android/hal-audio: Fix not handling EINTR errors
From: Luiz Augusto von Dentz @ 2014-01-28  6:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1390889820-24709-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

If the kernel interrupts us while writting just try again.
---
 android/hal-audio.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 8d737ad..2ca6289 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -399,7 +399,7 @@ static void sbc_resume(void *codec_data)
 	sbc_data->frames_sent = 0;
 }
 
-static void write_media_packet(int fd, struct sbc_data *sbc_data,
+static int write_media_packet(int fd, struct sbc_data *sbc_data,
 				struct media_packet *mp, size_t data_len)
 {
 	struct timespec cur;
@@ -407,10 +407,13 @@ static void write_media_packet(int fd, struct sbc_data *sbc_data,
 	unsigned expected_frames;
 	int ret;
 
-	ret = write(fd, mp, sizeof(*mp) + data_len);
-	if (ret < 0) {
-		int err = errno;
-		error("SBC: failed to write data: %d (%s)", err, strerror(err));
+	while (true) {
+		ret = write(fd, mp, sizeof(*mp) + data_len);
+		if (ret >= 0)
+			break;
+
+		if (errno != EINTR)
+			return -errno;
 	}
 
 	sbc_data->frames_sent += mp->payload.frame_count;
@@ -432,6 +435,8 @@ static void write_media_packet(int fd, struct sbc_data *sbc_data,
 	if (sbc_data->frames_sent >= expected_frames)
 		usleep(sbc_data->frame_duration *
 				mp->payload.frame_count);
+
+	return ret;
 }
 
 static ssize_t sbc_write_data(void *codec_data, const void *buffer,
@@ -474,7 +479,9 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
 		 */
 		if (mp->payload.frame_count == sbc_data->frames_per_packet ||
 				bytes == consumed) {
-			write_media_packet(fd, sbc_data, mp, encoded);
+			ret = write_media_packet(fd, sbc_data, mp, encoded);
+			if (ret < 0)
+				return ret;
 
 			encoded = 0;
 			free_space = sbc_data->out_buf_size - sizeof(*mp);
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH BlueZ v2 2/4] android/AVDTP: Make stream channel priority 5
From: Luiz Augusto von Dentz @ 2014-01-28  6:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1390889820-24709-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes channel priority 5 so it has higher priority than regular
traffic but less than signalling channel.
---
 android/avdtp.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/android/avdtp.c b/android/avdtp.c
index 4cfffc8..23fe519 100644
--- a/android/avdtp.c
+++ b/android/avdtp.c
@@ -2833,10 +2833,19 @@ gboolean avdtp_stream_set_transport(struct avdtp_stream *stream, int fd,
 						size_t imtu, size_t omtu)
 {
 	GIOChannel *io;
+	int priority;
 
 	if (stream != stream->session->pending_open)
 		return FALSE;
 
+	priority = 5;
+	if (setsockopt(fd, SOL_SOCKET, SO_PRIORITY, &priority,
+						sizeof(priority)) < 0) {
+		error("setsockopt(SO_PRIORITY): %s (%d)", strerror(errno),
+									errno);
+		return FALSE;
+	}
+
 	io = g_io_channel_unix_new(fd);
 
 	handle_transport_connect(stream->session, io, imtu, omtu);
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH BlueZ v2 1/4] android/AVDTP: Make signalling channel priority 6
From: Luiz Augusto von Dentz @ 2014-01-28  6:16 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes signalling priority 6 so it can push commands before the
stream channel, without this the stream channel may be schedule
first and cause the signalling commands to timeout while waiting a slot.
---
v2: Return error if writes fails since that probably means the socket has been
disconnected, also makes code setting socket to blocking a bit cleaner.

 android/avdtp.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/android/avdtp.c b/android/avdtp.c
index 4abcd75..4cfffc8 100644
--- a/android/avdtp.c
+++ b/android/avdtp.c
@@ -2057,6 +2057,7 @@ struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 	struct avdtp *session;
 	GIOCondition cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
 	int new_fd;
+	int priority;
 
 	new_fd = dup(fd);
 	if (new_fd < 0) {
@@ -2064,6 +2065,14 @@ struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 		return NULL;
 	}
 
+	priority = 6;
+	if (setsockopt(new_fd, SOL_SOCKET, SO_PRIORITY,
+			(const void *) &priority, sizeof(priority)) < 0) {
+		error("setsockopt(SO_PRIORITY): %s (%d)", strerror(errno),
+									errno);
+		return NULL;
+	}
+
 	session = g_new0(struct avdtp, 1);
 	session->io = g_io_channel_unix_new(new_fd);
 	session->version = version;
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH BlueZ 4/4] android/hal-audio: Set stream fd to blocking
From: Luiz Augusto von Dentz @ 2014-01-27 23:41 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1390866110-16184-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes the stream to block on io operation so it does not return
EAGAIN on syscall such as write.
---
 android/hal-audio.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 8a0737b..103e174 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -25,6 +25,7 @@
 #include <sys/un.h>
 #include <unistd.h>
 #include <arpa/inet.h>
+#include <fcntl.h>
 
 #include <hardware/audio.h>
 #include <hardware/hardware.h>
@@ -1159,8 +1160,13 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 	if (!preset || fd < 0)
 		goto fail;
 
-	out->ep->fd = fd;
+	if (fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK) < 0) {
+		error("fcntl(F_SETFL, ~O_NONBLOCK): %s (%d)", strerror(errno),
+									errno);
+		goto fail;
+	}
 
+	out->ep->fd = fd;
 	codec = out->ep->codec;
 
 	codec->init(preset, mtu, &out->ep->codec_data);
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH BlueZ 3/4] android/hal-audio: Fix not handling EINTR errors
From: Luiz Augusto von Dentz @ 2014-01-27 23:41 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1390866110-16184-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

If the kernel interrupts us while writting just try again.
---
 android/hal-audio.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 8d737ad..8a0737b 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -399,7 +399,7 @@ static void sbc_resume(void *codec_data)
 	sbc_data->frames_sent = 0;
 }
 
-static void write_media_packet(int fd, struct sbc_data *sbc_data,
+static int write_media_packet(int fd, struct sbc_data *sbc_data,
 				struct media_packet *mp, size_t data_len)
 {
 	struct timespec cur;
@@ -407,10 +407,13 @@ static void write_media_packet(int fd, struct sbc_data *sbc_data,
 	unsigned expected_frames;
 	int ret;
 
-	ret = write(fd, mp, sizeof(*mp) + data_len);
-	if (ret < 0) {
-		int err = errno;
-		error("SBC: failed to write data: %d (%s)", err, strerror(err));
+	while (true) {
+		ret = write(fd, mp, sizeof(*mp) + data_len);
+		if (ret >= 0)
+			break;
+
+		if (errno != EINTR)
+			return -errno;
 	}
 
 	sbc_data->frames_sent += mp->payload.frame_count;
@@ -432,6 +435,8 @@ static void write_media_packet(int fd, struct sbc_data *sbc_data,
 	if (sbc_data->frames_sent >= expected_frames)
 		usleep(sbc_data->frame_duration *
 				mp->payload.frame_count);
+
+	return ret;
 }
 
 static ssize_t sbc_write_data(void *codec_data, const void *buffer,
@@ -474,7 +479,12 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
 		 */
 		if (mp->payload.frame_count == sbc_data->frames_per_packet ||
 				bytes == consumed) {
-			write_media_packet(fd, sbc_data, mp, encoded);
+			ret = write_media_packet(fd, sbc_data, mp, encoded);
+			if (ret < 0) {
+				error("SBC: failed to write data: %d (%s)", ret,
+								strerror(ret));
+				break;
+			}
 
 			encoded = 0;
 			free_space = sbc_data->out_buf_size - sizeof(*mp);
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH BlueZ 2/4] android/AVDTP: Make stream channel priority 5
From: Luiz Augusto von Dentz @ 2014-01-27 23:41 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1390866110-16184-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes channel priority 5 so it has higher priority than regular
traffic but less than signalling channel.
---
 android/avdtp.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/android/avdtp.c b/android/avdtp.c
index 4cfffc8..23fe519 100644
--- a/android/avdtp.c
+++ b/android/avdtp.c
@@ -2833,10 +2833,19 @@ gboolean avdtp_stream_set_transport(struct avdtp_stream *stream, int fd,
 						size_t imtu, size_t omtu)
 {
 	GIOChannel *io;
+	int priority;
 
 	if (stream != stream->session->pending_open)
 		return FALSE;
 
+	priority = 5;
+	if (setsockopt(fd, SOL_SOCKET, SO_PRIORITY, &priority,
+						sizeof(priority)) < 0) {
+		error("setsockopt(SO_PRIORITY): %s (%d)", strerror(errno),
+									errno);
+		return FALSE;
+	}
+
 	io = g_io_channel_unix_new(fd);
 
 	handle_transport_connect(stream->session, io, imtu, omtu);
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH BlueZ 1/4] android/AVDTP: Make signalling channel priority 6
From: Luiz Augusto von Dentz @ 2014-01-27 23:41 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes signalling priority 6 so it can push commands before the
stream channel, without this the stream channel may be schedule
first and cause the signalling commands to timeout while waiting a slot.
---
 android/avdtp.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/android/avdtp.c b/android/avdtp.c
index 4abcd75..4cfffc8 100644
--- a/android/avdtp.c
+++ b/android/avdtp.c
@@ -2057,6 +2057,7 @@ struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 	struct avdtp *session;
 	GIOCondition cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
 	int new_fd;
+	int priority;
 
 	new_fd = dup(fd);
 	if (new_fd < 0) {
@@ -2064,6 +2065,14 @@ struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 		return NULL;
 	}
 
+	priority = 6;
+	if (setsockopt(new_fd, SOL_SOCKET, SO_PRIORITY,
+			(const void *) &priority, sizeof(priority)) < 0) {
+		error("setsockopt(SO_PRIORITY): %s (%d)", strerror(errno),
+									errno);
+		return NULL;
+	}
+
 	session = g_new0(struct avdtp, 1);
 	session->io = g_io_channel_unix_new(new_fd);
 	session->version = version;
-- 
1.8.4.2


^ permalink raw reply related

* Re: [PATCH 3/3] Bluetooth: Fix disconnecting L2CAP when a credits overflow occurs
From: Marcel Holtmann @ 2014-01-27 23:36 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: BlueZ development
In-Reply-To: <1390864295-4259-3-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> The L2CAP specification requires us to disconnect an L2CAP channel if
> the remote side gives us credits beyond 65535. This patch makes sure we
> disconnect the channel in such a situation.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/l2cap_core.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel


^ 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