* [PATCH BlueZ 1/3] avctp: Don't queue control request without callback
@ 2017-10-26 12:02 Luiz Augusto von Dentz
2017-10-26 12:02 ` [PATCH BlueZ 2/3] avctp: Reintroduce a timeout for the browsing channel Luiz Augusto von Dentz
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-26 12:02 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
If no callback is set that means the request don't care about the
response, so instead of putting such request into the pending queue
this proceeds to send them immediately.
---
profiles/audio/avctp.c | 67 +++++++++++++++++++++++++++++++-------------------
1 file changed, 42 insertions(+), 25 deletions(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index daf39a782..f8ec19c9c 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -617,6 +617,39 @@ static void avctp_set_state(struct avctp *session, avctp_state_t new_state,
}
}
+static uint8_t chan_get_transaction(struct avctp_channel *chan)
+{
+ GSList *l, *tmp;
+ uint8_t transaction;
+
+ if (!chan->processed)
+ goto done;
+
+ tmp = g_slist_copy(chan->processed);
+
+ /* Find first unused transaction id */
+ for (l = tmp; l; l = g_slist_next(l)) {
+ struct avctp_pending_req *req = l->data;
+
+ if (req->transaction == chan->transaction) {
+ chan->transaction++;
+ chan->transaction %= 16;
+ tmp = g_slist_delete_link(tmp, l);
+ l = tmp;
+ }
+ }
+
+ g_slist_free(tmp);
+
+done:
+ transaction = chan->transaction;
+
+ chan->transaction++;
+ chan->transaction %= 16;
+
+ return transaction;
+}
+
static int avctp_send(struct avctp_channel *control, uint8_t transaction,
uint8_t cr, uint8_t code,
uint8_t subunit, uint8_t opcode,
@@ -643,6 +676,9 @@ static int avctp_send(struct avctp_channel *control, uint8_t transaction,
avctp = (void *) control->buffer;
avc = (void *) avctp + sizeof(*avctp);
+ if (transaction > 16)
+ transaction = chan_get_transaction(control);
+
avctp->transaction = transaction;
avctp->packet_type = AVCTP_PACKET_SINGLE;
avctp->cr = cr;
@@ -1556,38 +1592,14 @@ static struct avctp_pending_req *pending_create(struct avctp_channel *chan,
GDestroyNotify destroy)
{
struct avctp_pending_req *p;
- GSList *l, *tmp;
-
- if (!chan->processed)
- goto done;
-
- tmp = g_slist_copy(chan->processed);
-
- /* Find first unused transaction id */
- for (l = tmp; l; l = g_slist_next(l)) {
- struct avctp_pending_req *req = l->data;
-
- if (req->transaction == chan->transaction) {
- chan->transaction++;
- chan->transaction %= 16;
- tmp = g_slist_delete_link(tmp, l);
- l = tmp;
- }
- }
- g_slist_free(tmp);
-
-done:
p = g_new0(struct avctp_pending_req, 1);
p->chan = chan;
- p->transaction = chan->transaction;
+ p->transaction = chan_get_transaction(chan);
p->process = process;
p->data = data;
p->destroy = destroy;
- chan->transaction++;
- chan->transaction %= 16;
-
return p;
}
@@ -1603,6 +1615,11 @@ static int avctp_send_req(struct avctp *session, uint8_t code,
if (control == NULL)
return -ENOTCONN;
+ /* If the request set a callback send it directly */
+ if (!func)
+ return avctp_send(session->control, -1, AVCTP_COMMAND,
+ code, subunit, opcode, operands, operand_count);
+
req = g_new0(struct avctp_control_req, 1);
req->code = code;
req->subunit = subunit;
--
2.13.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH BlueZ 2/3] avctp: Reintroduce a timeout for the browsing channel
2017-10-26 12:02 [PATCH BlueZ 1/3] avctp: Don't queue control request without callback Luiz Augusto von Dentz
@ 2017-10-26 12:02 ` Luiz Augusto von Dentz
2017-10-26 12:02 ` [PATCH BlueZ 3/3] avctp: Retry control PDU if timeout Luiz Augusto von Dentz
2017-10-30 12:55 ` [PATCH BlueZ 1/3] avctp: Don't queue control request without callback Luiz Augusto von Dentz
2 siblings, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-26 12:02 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
The spec suggests to rely on the link supervision timeout to detect
when the remote is not responding but this ignores the fact that due
to a bug or bad implementation stacks may ignore the request causing
the whole request queue to stall.
---
profiles/audio/avctp.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index f8ec19c9c..174f401d6 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -62,6 +62,7 @@
#define AVC_PRESS_TIMEOUT 2
#define CONTROL_TIMEOUT AVC_PRESS_TIMEOUT
+#define BROWSING_TIMEOUT 10
#define QUIRK_NO_RELEASE 1 << 0
@@ -810,9 +811,17 @@ static int process_browsing(void *data)
{
struct avctp_browsing_req *req = data;
struct avctp_pending_req *p = req->p;
+ int ret;
- return avctp_browsing_send(p->chan, p->transaction, AVCTP_COMMAND,
+ ret = avctp_browsing_send(p->chan, p->transaction, AVCTP_COMMAND,
req->operands, req->operand_count);
+ if (ret < 0)
+ return ret;
+
+ p->timeout = g_timeout_add_seconds(BROWSING_TIMEOUT, req_timeout,
+ p->chan);
+
+ return 0;
}
static gboolean process_queue(void *user_data)
--
2.13.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH BlueZ 3/3] avctp: Retry control PDU if timeout
2017-10-26 12:02 [PATCH BlueZ 1/3] avctp: Don't queue control request without callback Luiz Augusto von Dentz
2017-10-26 12:02 ` [PATCH BlueZ 2/3] avctp: Reintroduce a timeout for the browsing channel Luiz Augusto von Dentz
@ 2017-10-26 12:02 ` Luiz Augusto von Dentz
2017-10-30 12:55 ` [PATCH BlueZ 1/3] avctp: Don't queue control request without callback Luiz Augusto von Dentz
2 siblings, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-26 12:02 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This attempts to resend a control PDU is not passthrough as the control
channel is considered unreliable packets may be lost.
---
profiles/audio/avctp.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 174f401d6..0d395814c 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -157,6 +157,7 @@ struct avctp_pending_req {
struct avctp_channel *chan;
uint8_t transaction;
guint timeout;
+ bool retry;
int err;
avctp_process_cb process;
void *data;
@@ -775,9 +776,16 @@ static gboolean req_timeout(gpointer user_data)
struct avctp_channel *chan = user_data;
struct avctp_pending_req *p = chan->p;
- DBG("transaction %u", p->transaction);
+ DBG("transaction %u retry %s", p->transaction, p->retry ? "true" :
+ "false");
p->timeout = 0;
+
+ if (p->retry) {
+ p->process(p->data);
+ return FALSE;
+ }
+
p->err = -ETIMEDOUT;
pending_destroy(p, NULL);
@@ -801,6 +809,9 @@ static int process_control(void *data)
if (ret < 0)
return ret;
+ if (req->op != AVC_OP_PASSTHROUGH)
+ p->retry = !p->retry;
+
p->timeout = g_timeout_add_seconds(CONTROL_TIMEOUT, req_timeout,
p->chan);
--
2.13.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH BlueZ 1/3] avctp: Don't queue control request without callback
2017-10-26 12:02 [PATCH BlueZ 1/3] avctp: Don't queue control request without callback Luiz Augusto von Dentz
2017-10-26 12:02 ` [PATCH BlueZ 2/3] avctp: Reintroduce a timeout for the browsing channel Luiz Augusto von Dentz
2017-10-26 12:02 ` [PATCH BlueZ 3/3] avctp: Retry control PDU if timeout Luiz Augusto von Dentz
@ 2017-10-30 12:55 ` Luiz Augusto von Dentz
2 siblings, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2017-10-30 12:55 UTC (permalink / raw)
To: linux-bluetooth
Hi,
On Thu, Oct 26, 2017 at 3:02 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> If no callback is set that means the request don't care about the
> response, so instead of putting such request into the pending queue
> this proceeds to send them immediately.
> ---
> profiles/audio/avctp.c | 67 +++++++++++++++++++++++++++++++-------------------
> 1 file changed, 42 insertions(+), 25 deletions(-)
>
> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> index daf39a782..f8ec19c9c 100644
> --- a/profiles/audio/avctp.c
> +++ b/profiles/audio/avctp.c
> @@ -617,6 +617,39 @@ static void avctp_set_state(struct avctp *session, avctp_state_t new_state,
> }
> }
>
> +static uint8_t chan_get_transaction(struct avctp_channel *chan)
> +{
> + GSList *l, *tmp;
> + uint8_t transaction;
> +
> + if (!chan->processed)
> + goto done;
> +
> + tmp = g_slist_copy(chan->processed);
> +
> + /* Find first unused transaction id */
> + for (l = tmp; l; l = g_slist_next(l)) {
> + struct avctp_pending_req *req = l->data;
> +
> + if (req->transaction == chan->transaction) {
> + chan->transaction++;
> + chan->transaction %= 16;
> + tmp = g_slist_delete_link(tmp, l);
> + l = tmp;
> + }
> + }
> +
> + g_slist_free(tmp);
> +
> +done:
> + transaction = chan->transaction;
> +
> + chan->transaction++;
> + chan->transaction %= 16;
> +
> + return transaction;
> +}
> +
> static int avctp_send(struct avctp_channel *control, uint8_t transaction,
> uint8_t cr, uint8_t code,
> uint8_t subunit, uint8_t opcode,
> @@ -643,6 +676,9 @@ static int avctp_send(struct avctp_channel *control, uint8_t transaction,
> avctp = (void *) control->buffer;
> avc = (void *) avctp + sizeof(*avctp);
>
> + if (transaction > 16)
> + transaction = chan_get_transaction(control);
> +
> avctp->transaction = transaction;
> avctp->packet_type = AVCTP_PACKET_SINGLE;
> avctp->cr = cr;
> @@ -1556,38 +1592,14 @@ static struct avctp_pending_req *pending_create(struct avctp_channel *chan,
> GDestroyNotify destroy)
> {
> struct avctp_pending_req *p;
> - GSList *l, *tmp;
> -
> - if (!chan->processed)
> - goto done;
> -
> - tmp = g_slist_copy(chan->processed);
> -
> - /* Find first unused transaction id */
> - for (l = tmp; l; l = g_slist_next(l)) {
> - struct avctp_pending_req *req = l->data;
> -
> - if (req->transaction == chan->transaction) {
> - chan->transaction++;
> - chan->transaction %= 16;
> - tmp = g_slist_delete_link(tmp, l);
> - l = tmp;
> - }
> - }
>
> - g_slist_free(tmp);
> -
> -done:
> p = g_new0(struct avctp_pending_req, 1);
> p->chan = chan;
> - p->transaction = chan->transaction;
> + p->transaction = chan_get_transaction(chan);
> p->process = process;
> p->data = data;
> p->destroy = destroy;
>
> - chan->transaction++;
> - chan->transaction %= 16;
> -
> return p;
> }
>
> @@ -1603,6 +1615,11 @@ static int avctp_send_req(struct avctp *session, uint8_t code,
> if (control == NULL)
> return -ENOTCONN;
>
> + /* If the request set a callback send it directly */
> + if (!func)
> + return avctp_send(session->control, -1, AVCTP_COMMAND,
> + code, subunit, opcode, operands, operand_count);
> +
> req = g_new0(struct avctp_control_req, 1);
> req->code = code;
> req->subunit = subunit;
> --
> 2.13.6
Applied.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-10-30 12:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-26 12:02 [PATCH BlueZ 1/3] avctp: Don't queue control request without callback Luiz Augusto von Dentz
2017-10-26 12:02 ` [PATCH BlueZ 2/3] avctp: Reintroduce a timeout for the browsing channel Luiz Augusto von Dentz
2017-10-26 12:02 ` [PATCH BlueZ 3/3] avctp: Retry control PDU if timeout Luiz Augusto von Dentz
2017-10-30 12:55 ` [PATCH BlueZ 1/3] avctp: Don't queue control request without callback Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.