Some headsets try connect AVCTP during our outgoing AVCTP connection. In effect
both connections are rejected, and session is closed. Daemon logs:
localhost.localdomain bluetoothd[11189]: Control: Refusing unexpected connect
localhost.localdomain bluetoothd[11189]: connect error: Connection refused (111)
In BTSNOOP:
< ACL Data TX: Handle 256 flags 0x00 dlen 12
L2CAP: Connection Request (0x02) ident 11 len 4
PSM: 23 (0x0017)
Source CID: 68
> HCI Event: Number of Completed Packets (0x13) plen 5
Num handles: 1
Handle: 256
Count: 1
> ACL Data RX: Handle 256 flags 0x02 dlen 16
L2CAP: Connection Response (0x03) ident 11 len 8
Destination CID: 69
Source CID: 68
Result: Connection pending (0x0001)
Status: Authorization pending (0x0002)
< ACL Data TX: Handle 256 flags 0x00 dlen 12
L2CAP: Disconnection Request (0x06) ident 12 len 4
Destination CID: 64
Source CID: 64
> HCI Event: Number of Completed Packets (0x13) plen 5
Num handles: 1
Handle: 256
Count: 1
> ACL Data RX: Handle 256 flags 0x02 dlen 12
L2CAP: Disconnection Response (0x07) ident 12 len 4
Destination CID: 64
Source CID: 64
> ACL Data RX: Handle 256 flags 0x02 dlen 12
L2CAP: Connection Request (0x02) ident 9 len 4
PSM: 23 (0x0017)
Source CID: 70
< ACL Data TX: Handle 256 flags 0x00 dlen 16
L2CAP: Connection Response (0x03) ident 9 len 8
Destination CID: 64
Source CID: 70
Result: Connection pending (0x0001)
Status: Authorization pending (0x0002)
< ACL Data TX: Handle 256 flags 0x00 dlen 16
L2CAP: Connection Response (0x03) ident 9 len 8
Destination CID: 64
Source CID: 70
Result: Connection refused - security block (0x0003)
Status: No further information available (0x0000)
> ACL Data RX: Handle 256 flags 0x02 dlen 16
L2CAP: Connection Response (0x03) ident 11 len 8
Destination CID: 0
Source CID: 68
Result: Connection refused - no resources available (0x0004)
Status: No further information available (0x0000)
With provided fix, we accept incoming connection and close outgoing connection.
If accepting won't succeed, we try reconnect as initiator. Btsnoop after fix:
< ACL Data TX: Handle 256 flags 0x00 dlen 12
L2CAP: Connection Request (0x02) ident 11 len 4
PSM: 23 (0x0017)
Source CID: 67
> HCI Event: Number of Completed Packets (0x13) plen 5
Num handles: 1
Handle: 256
Count: 1
> ACL Data RX: Handle 256 flags 0x02 dlen 16
L2CAP: Connection Response (0x03) ident 11 len 8
Destination CID: 87
Source CID: 67
Result: Connection pending (0x0001)
Status: Authorization pending (0x0002)
> ACL Data RX: Handle 256 flags 0x02 dlen 12
L2CAP: Connection Request (0x02) ident 5 len 4
PSM: 23 (0x0017)
Source CID: 88
< ACL Data TX: Handle 256 flags 0x00 dlen 16
L2CAP: Connection Response (0x03) ident 5 len 8
Destination CID: 68
Source CID: 88
Result: Connection pending (0x0001)
Status: Authorization pending (0x0002)
< ACL Data TX: Handle 256 flags 0x00 dlen 16
L2CAP: Connection Response (0x03) ident 5 len 8
Destination CID: 68
Source CID: 88
Result: Connection successful (0x0000)
Status: No further information available (0x0000)
< ACL Data TX: Handle 256 flags 0x00 dlen 12
L2CAP: Configure Request (0x04) ident 12 len 4
Destination CID: 88
Flags: 0x0000
> HCI Event: Number of Completed Packets (0x13) plen 5
Num handles: 1
Handle: 256
Count: 1
> HCI Event: Number of Completed Packets (0x13) plen 5
Num handles: 1
Handle: 256
Count: 1
> HCI Event: Number of Completed Packets (0x13) plen 5
Num handles: 1
Handle: 256
Count: 1
> ACL Data RX: Handle 256 flags 0x02 dlen 16
L2CAP: Connection Response (0x03) ident 11 len 8
Destination CID: 0
Source CID: 67
Result: Connection refused - no resources available (0x0004)
Status: No further information available (0x0000)
Issue reproduced with NOKIA BH-505 and Motorola HT820.
Comments are welcome.
Marcin Kraglak (1):
audio/avctp: Accept incoming connection in case of colission
profiles/audio/avctp.c | 38 ++++++++++++++++++++++++++++++++++++--
1 file changed, 36 insertions(+), 2 deletions(-)
--
2.1.0
S
Hi Szymon,
On Fri, Feb 27, 2015 at 12:37 PM, Szymon Janc <[email protected]> wrote:
> Hi Marcin,
>
> On Friday 27 of February 2015 09:01:12 Marcin Kraglak wrote:
>> Accept incoming connection to Control Channel in case of colission.
>> If accepting connection won't succeed, try reconnect.
>> There is no need to request authorization while user has been connecting
>> to service in meantime.
>> Issue was found during tests with NOKIA BH-505 and Motorola HT-820.
>> ---
>> profiles/audio/avctp.c | 38 ++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
>> index 57071b5..8d457f4 100644
>> --- a/profiles/audio/avctp.c
>> +++ b/profiles/audio/avctp.c
>> @@ -203,6 +203,7 @@ struct avctp {
>> uint8_t key_quirks[256];
>> struct key_pressed key;
>> bool initiator;
>> + bool reconnect;
>> };
>>
>> struct avctp_passthrough_handler {
>> @@ -1219,11 +1220,22 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
>> GError *gerr = NULL;
>>
>> if (err) {
>> + struct btd_device *device = session->device;
>> + bool reconnect = session->reconnect;
>> +
>> avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
>> error("%s", err->message);
>> +
>> + if (!reconnect)
>> + return;
>> +
>> + DBG("Control: Attempting to reconnect");
>> + avctp_connect(device);
>> return;
>> }
>>
>> + session->reconnect = false;
>> +
>> bt_io_get(chan, &gerr,
>> BT_IO_OPT_DEST, &address,
>> BT_IO_OPT_IMTU, &imtu,
>> @@ -1346,10 +1358,32 @@ static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
>> {
>> const bdaddr_t *src;
>> const bdaddr_t *dst;
>> + GError *err = NULL;
>>
>> if (session->control != NULL) {
>> - error("Control: Refusing unexpected connect");
>> - g_io_channel_shutdown(chan, TRUE, NULL);
>> + if (session->state != AVCTP_STATE_CONNECTING) {
>> + error("Control: Refusing unexpected connect");
>> + g_io_channel_shutdown(chan, TRUE, NULL);
>> + return;
>> + }
>> +
>> + /* Cancel outgoing connection and accept incoming */
>> + DBG("Control: Accepting unexpected connect from remote");
>> + g_io_channel_shutdown(session->control->io, TRUE, NULL);
>> + g_io_channel_unref(session->control->io);
>> + session->control->io = NULL;
>> +
>> + if (!bt_io_accept(chan, avctp_connect_cb, session, NULL,
>> + &err)) {
>> + error("bt_io_accept: %s", err->message);
>> + g_error_free(err);
>> + avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
>> + return;
>> + }
>> +
>> + session->reconnect = true;
>> + session->control->io = g_io_channel_ref(chan);
>> +
>> return;
>> }
>
> I think we should go with random timer as required by AVRCP specification
>
> p.4.1.1:
> "If both devices open an AVCTP channel at the same time both channels shall be
> closed and each device shall wait a random time (not more than 1 s and not less
> than 100ms) and then try to open the AVCTP channel again. If it is known which
> device is the master that device can re-try at once."
That and it is probably a better idea to leave this to policy to
re-try as it is already doing for A2DP.
--
Luiz Augusto von Dentz
Hi Marcin,
On Friday 27 of February 2015 09:01:12 Marcin Kraglak wrote:
> Accept incoming connection to Control Channel in case of colission.
> If accepting connection won't succeed, try reconnect.
> There is no need to request authorization while user has been connecting
> to service in meantime.
> Issue was found during tests with NOKIA BH-505 and Motorola HT-820.
> ---
> profiles/audio/avctp.c | 38 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> index 57071b5..8d457f4 100644
> --- a/profiles/audio/avctp.c
> +++ b/profiles/audio/avctp.c
> @@ -203,6 +203,7 @@ struct avctp {
> uint8_t key_quirks[256];
> struct key_pressed key;
> bool initiator;
> + bool reconnect;
> };
>
> struct avctp_passthrough_handler {
> @@ -1219,11 +1220,22 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
> GError *gerr = NULL;
>
> if (err) {
> + struct btd_device *device = session->device;
> + bool reconnect = session->reconnect;
> +
> avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
> error("%s", err->message);
> +
> + if (!reconnect)
> + return;
> +
> + DBG("Control: Attempting to reconnect");
> + avctp_connect(device);
> return;
> }
>
> + session->reconnect = false;
> +
> bt_io_get(chan, &gerr,
> BT_IO_OPT_DEST, &address,
> BT_IO_OPT_IMTU, &imtu,
> @@ -1346,10 +1358,32 @@ static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
> {
> const bdaddr_t *src;
> const bdaddr_t *dst;
> + GError *err = NULL;
>
> if (session->control != NULL) {
> - error("Control: Refusing unexpected connect");
> - g_io_channel_shutdown(chan, TRUE, NULL);
> + if (session->state != AVCTP_STATE_CONNECTING) {
> + error("Control: Refusing unexpected connect");
> + g_io_channel_shutdown(chan, TRUE, NULL);
> + return;
> + }
> +
> + /* Cancel outgoing connection and accept incoming */
> + DBG("Control: Accepting unexpected connect from remote");
> + g_io_channel_shutdown(session->control->io, TRUE, NULL);
> + g_io_channel_unref(session->control->io);
> + session->control->io = NULL;
> +
> + if (!bt_io_accept(chan, avctp_connect_cb, session, NULL,
> + &err)) {
> + error("bt_io_accept: %s", err->message);
> + g_error_free(err);
> + avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
> + return;
> + }
> +
> + session->reconnect = true;
> + session->control->io = g_io_channel_ref(chan);
> +
> return;
> }
I think we should go with random timer as required by AVRCP specification
p.4.1.1:
"If both devices open an AVCTP channel at the same time both channels shall be
closed and each device shall wait a random time (not more than 1 s and not less
than 100ms) and then try to open the AVCTP channel again. If it is known which
device is the master that device can re-try at once."
--
Best regards,
Szymon Janc
Accept incoming connection to Control Channel in case of colission.
If accepting connection won't succeed, try reconnect.
There is no need to request authorization while user has been connecting
to service in meantime.
Issue was found during tests with NOKIA BH-505 and Motorola HT-820.
---
profiles/audio/avctp.c | 38 ++++++++++++++++++++++++++++++++++++--
1 file changed, 36 insertions(+), 2 deletions(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 57071b5..8d457f4 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -203,6 +203,7 @@ struct avctp {
uint8_t key_quirks[256];
struct key_pressed key;
bool initiator;
+ bool reconnect;
};
struct avctp_passthrough_handler {
@@ -1219,11 +1220,22 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
GError *gerr = NULL;
if (err) {
+ struct btd_device *device = session->device;
+ bool reconnect = session->reconnect;
+
avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
error("%s", err->message);
+
+ if (!reconnect)
+ return;
+
+ DBG("Control: Attempting to reconnect");
+ avctp_connect(device);
return;
}
+ session->reconnect = false;
+
bt_io_get(chan, &gerr,
BT_IO_OPT_DEST, &address,
BT_IO_OPT_IMTU, &imtu,
@@ -1346,10 +1358,32 @@ static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
{
const bdaddr_t *src;
const bdaddr_t *dst;
+ GError *err = NULL;
if (session->control != NULL) {
- error("Control: Refusing unexpected connect");
- g_io_channel_shutdown(chan, TRUE, NULL);
+ if (session->state != AVCTP_STATE_CONNECTING) {
+ error("Control: Refusing unexpected connect");
+ g_io_channel_shutdown(chan, TRUE, NULL);
+ return;
+ }
+
+ /* Cancel outgoing connection and accept incoming */
+ DBG("Control: Accepting unexpected connect from remote");
+ g_io_channel_shutdown(session->control->io, TRUE, NULL);
+ g_io_channel_unref(session->control->io);
+ session->control->io = NULL;
+
+ if (!bt_io_accept(chan, avctp_connect_cb, session, NULL,
+ &err)) {
+ error("bt_io_accept: %s", err->message);
+ g_error_free(err);
+ avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
+ return;
+ }
+
+ session->reconnect = true;
+ session->control->io = g_io_channel_ref(chan);
+
return;
}
--
2.1.0
Hi Marcin,
On Mon, Mar 2, 2015 at 8:59 AM, Marcin Kraglak <[email protected]> wrote:
> Hi Luiz,
>
> Why do you want to move it to policy? As I understand policy is a way
> we want to improve user-experience, such connecting to default
> profiles etc., it's about channel establishment.
It is the policy module that takes care of connection logic, it is it
that does start the connection attempt once A2DP connects, in fact all
the policy does is connection policy so it can keep track what should
be connected or not and deal with reconnection logic, etc.
> On 27 February 2015 at 15:07, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Szymon,
>>
>> On Fri, Feb 27, 2015 at 12:37 PM, Szymon Janc <[email protected]> wrote:
>>> Hi Marcin,
>>>
>>> On Friday 27 of February 2015 09:01:12 Marcin Kraglak wrote:
>>>> Accept incoming connection to Control Channel in case of colission.
>>>> If accepting connection won't succeed, try reconnect.
>>>> There is no need to request authorization while user has been connecting
>>>> to service in meantime.
>>>> Issue was found during tests with NOKIA BH-505 and Motorola HT-820.
>>>> ---
>>>> profiles/audio/avctp.c | 38 ++++++++++++++++++++++++++++++++++++--
>>>> 1 file changed, 36 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
>>>> index 57071b5..8d457f4 100644
>>>> --- a/profiles/audio/avctp.c
>>>> +++ b/profiles/audio/avctp.c
>>>> @@ -203,6 +203,7 @@ struct avctp {
>>>> uint8_t key_quirks[256];
>>>> struct key_pressed key;
>>>> bool initiator;
>>>> + bool reconnect;
>>>> };
>>>>
>>>> struct avctp_passthrough_handler {
>>>> @@ -1219,11 +1220,22 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
>>>> GError *gerr = NULL;
>>>>
>>>> if (err) {
>>>> + struct btd_device *device = session->device;
>>>> + bool reconnect = session->reconnect;
>>>> +
>>>> avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
>>>> error("%s", err->message);
>>>> +
>>>> + if (!reconnect)
>>>> + return;
>>>> +
>>>> + DBG("Control: Attempting to reconnect");
>>>> + avctp_connect(device);
>>>> return;
>>>> }
>>>>
>>>> + session->reconnect = false;
>>>> +
>>>> bt_io_get(chan, &gerr,
>>>> BT_IO_OPT_DEST, &address,
>>>> BT_IO_OPT_IMTU, &imtu,
>>>> @@ -1346,10 +1358,32 @@ static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
>>>> {
>>>> const bdaddr_t *src;
>>>> const bdaddr_t *dst;
>>>> + GError *err = NULL;
>>>>
>>>> if (session->control != NULL) {
>>>> - error("Control: Refusing unexpected connect");
>>>> - g_io_channel_shutdown(chan, TRUE, NULL);
>>>> + if (session->state != AVCTP_STATE_CONNECTING) {
>>>> + error("Control: Refusing unexpected connect");
>>>> + g_io_channel_shutdown(chan, TRUE, NULL);
>>>> + return;
>>>> + }
>>>> +
>>>> + /* Cancel outgoing connection and accept incoming */
>>>> + DBG("Control: Accepting unexpected connect from remote");
>>>> + g_io_channel_shutdown(session->control->io, TRUE, NULL);
>>>> + g_io_channel_unref(session->control->io);
>>>> + session->control->io = NULL;
>>>> +
>>>> + if (!bt_io_accept(chan, avctp_connect_cb, session, NULL,
>>>> + &err)) {
>>>> + error("bt_io_accept: %s", err->message);
>>>> + g_error_free(err);
>>>> + avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
>>>> + return;
>>>> + }
>>>> +
>>>> + session->reconnect = true;
>>>> + session->control->io = g_io_channel_ref(chan);
>>>> +
>>>> return;
>>>> }
>>>
>>> I think we should go with random timer as required by AVRCP specification
>>>
>>> p.4.1.1:
>>> "If both devices open an AVCTP channel at the same time both channels shall be
>>> closed and each device shall wait a random time (not more than 1 s and not less
>>> than 100ms) and then try to open the AVCTP channel again. If it is known which
>>> device is the master that device can re-try at once."
>>
>> That and it is probably a better idea to leave this to policy to
>> re-try as it is already doing for A2DP.
>>
>>
>> --
>> Luiz Augusto von Dentz
>
>
>
> --
> BR
> Marcin Kraglak
--
Luiz Augusto von Dentz
Hi Luiz,
Why do you want to move it to policy? As I understand policy is a way
we want to improve user-experience, such connecting to default
profiles etc., it's about channel establishment.
On 27 February 2015 at 15:07, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Szymon,
>
> On Fri, Feb 27, 2015 at 12:37 PM, Szymon Janc <[email protected]> wrote:
>> Hi Marcin,
>>
>> On Friday 27 of February 2015 09:01:12 Marcin Kraglak wrote:
>>> Accept incoming connection to Control Channel in case of colission.
>>> If accepting connection won't succeed, try reconnect.
>>> There is no need to request authorization while user has been connecting
>>> to service in meantime.
>>> Issue was found during tests with NOKIA BH-505 and Motorola HT-820.
>>> ---
>>> profiles/audio/avctp.c | 38 ++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 36 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
>>> index 57071b5..8d457f4 100644
>>> --- a/profiles/audio/avctp.c
>>> +++ b/profiles/audio/avctp.c
>>> @@ -203,6 +203,7 @@ struct avctp {
>>> uint8_t key_quirks[256];
>>> struct key_pressed key;
>>> bool initiator;
>>> + bool reconnect;
>>> };
>>>
>>> struct avctp_passthrough_handler {
>>> @@ -1219,11 +1220,22 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
>>> GError *gerr = NULL;
>>>
>>> if (err) {
>>> + struct btd_device *device = session->device;
>>> + bool reconnect = session->reconnect;
>>> +
>>> avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
>>> error("%s", err->message);
>>> +
>>> + if (!reconnect)
>>> + return;
>>> +
>>> + DBG("Control: Attempting to reconnect");
>>> + avctp_connect(device);
>>> return;
>>> }
>>>
>>> + session->reconnect = false;
>>> +
>>> bt_io_get(chan, &gerr,
>>> BT_IO_OPT_DEST, &address,
>>> BT_IO_OPT_IMTU, &imtu,
>>> @@ -1346,10 +1358,32 @@ static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
>>> {
>>> const bdaddr_t *src;
>>> const bdaddr_t *dst;
>>> + GError *err = NULL;
>>>
>>> if (session->control != NULL) {
>>> - error("Control: Refusing unexpected connect");
>>> - g_io_channel_shutdown(chan, TRUE, NULL);
>>> + if (session->state != AVCTP_STATE_CONNECTING) {
>>> + error("Control: Refusing unexpected connect");
>>> + g_io_channel_shutdown(chan, TRUE, NULL);
>>> + return;
>>> + }
>>> +
>>> + /* Cancel outgoing connection and accept incoming */
>>> + DBG("Control: Accepting unexpected connect from remote");
>>> + g_io_channel_shutdown(session->control->io, TRUE, NULL);
>>> + g_io_channel_unref(session->control->io);
>>> + session->control->io = NULL;
>>> +
>>> + if (!bt_io_accept(chan, avctp_connect_cb, session, NULL,
>>> + &err)) {
>>> + error("bt_io_accept: %s", err->message);
>>> + g_error_free(err);
>>> + avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
>>> + return;
>>> + }
>>> +
>>> + session->reconnect = true;
>>> + session->control->io = g_io_channel_ref(chan);
>>> +
>>> return;
>>> }
>>
>> I think we should go with random timer as required by AVRCP specification
>>
>> p.4.1.1:
>> "If both devices open an AVCTP channel at the same time both channels shall be
>> closed and each device shall wait a random time (not more than 1 s and not less
>> than 100ms) and then try to open the AVCTP channel again. If it is known which
>> device is the master that device can re-try at once."
>
> That and it is probably a better idea to leave this to policy to
> re-try as it is already doing for A2DP.
>
>
> --
> Luiz Augusto von Dentz
--
BR
Marcin Kraglak