2012-02-07 14:43:01

by Szymon Janc

Subject: [PATCH 1/2] Bluetooth: Fix possible missing I-Frame acknowledgement

Make l2cap_ertm_send return number of pending I-Frames transmitted
instead of all (pending + retransmitted) I-Frames transmitted.

As only pending I-Frames are considered as acknowledgement, this could
lead to situation when no ACK was sent in __l2cap_send_ack (if only
already transmitted I-Frames were retransmitted).

Signed-off-by: Szymon Janc <[email protected]>
---
net/bluetooth/l2cap_core.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 09cd860..5bb298d 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1448,8 +1448,10 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)

chan->next_tx_seq = __next_seq(chan, chan->next_tx_seq);

- if (bt_cb(skb)->retries == 1)
+ if (bt_cb(skb)->retries == 1) {
chan->unacked_frames++;
+ nsent++;
+ }

chan->frames_sent++;

@@ -1457,8 +1459,6 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
chan->tx_send_head = NULL;
else
chan->tx_send_head = skb_queue_next(&chan->tx_q, skb);
-
- nsent++;
}

return nsent;
--
on behalf of ST-Ericsson

2012-02-09 15:28:54

by Johan Hedberg

Subject: Re: [PATCH 1/2] Bluetooth: Fix possible missing I-Frame acknowledgement

Hi Szymon,

On Tue, Feb 07, 2012, Szymon Janc wrote:
> Make l2cap_ertm_send return number of pending I-Frames transmitted
> instead of all (pending + retransmitted) I-Frames transmitted.
>
> As only pending I-Frames are considered as acknowledgement, this could
> lead to situation when no ACK was sent in __l2cap_send_ack (if only
> already transmitted I-Frames were retransmitted).
>
> Signed-off-by: Szymon Janc <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)

Both patches have been applied to my bluetooth-next tree. Thanks.

Johan

2012-02-09 13:45:38

by Marcel Holtmann

Subject: Re: [PATCH 2/2] Bluetooth: Fix double acking I-Frames when sending pending I-Frames

Hi Szymon,

> Pending I-Frame(s) are considered as acknowledgement. To void double
> acking (via I-Frame and later via RR) clear ack timer when sending
> first pending I-Frame.
>
> Signed-off-by: Szymon Janc <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel

2012-02-09 13:44:45

by Marcel Holtmann

Subject: Re: [PATCH 1/2] Bluetooth: Fix possible missing I-Frame acknowledgement

Hi Szymon,

> Make l2cap_ertm_send return number of pending I-Frames transmitted
> instead of all (pending + retransmitted) I-Frames transmitted.
>
> As only pending I-Frames are considered as acknowledgement, this could
> lead to situation when no ACK was sent in __l2cap_send_ack (if only
> already transmitted I-Frames were retransmitted).
>
> Signed-off-by: Szymon Janc <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel

2012-02-08 16:40:58

by Gustavo Padovan

Subject: Re: [PATCH 1/2] Bluetooth: Fix possible missing I-Frame acknowledgement

Hi Szymon,

* Szymon Janc <[email protected]> [2012-02-07 15:43:01 +0100]:

> Make l2cap_ertm_send return number of pending I-Frames transmitted
> instead of all (pending + retransmitted) I-Frames transmitted.
>
> As only pending I-Frames are considered as acknowledgement, this could
> lead to situation when no ACK was sent in __l2cap_send_ack (if only
> already transmitted I-Frames were retransmitted).
>
> Signed-off-by: Szymon Janc <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 09cd860..5bb298d 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1448,8 +1448,10 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
>
> chan->next_tx_seq = __next_seq(chan, chan->next_tx_seq);
>
> - if (bt_cb(skb)->retries == 1)
> + if (bt_cb(skb)->retries == 1) {
> chan->unacked_frames++;
> + nsent++;
> + }
>
> chan->frames_sent++;
>
> @@ -1457,8 +1459,6 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
> chan->tx_send_head = NULL;
> else
> chan->tx_send_head = skb_queue_next(&chan->tx_q, skb);
> -
> - nsent++;
> }
>
> return nsent;


Acked-by: Gustavo F. Padovan <[email protected]>

Gustavo

2012-02-07 15:50:03

by Luiz Augusto von Dentz

Subject: Re: [PATCH 2/2] Bluetooth: Fix double acking I-Frames when sending pending I-Frames

Hi Szymon,

On Tue, Feb 7, 2012 at 5:26 PM, Ulisses Furquim <[email protected]> wrote:
> Hi Szymon,
>
> On Tue, Feb 7, 2012 at 12:43 PM, Szymon Janc <[email protected]> wrote:
>> Pending I-Frame(s) are considered as acknowledgement. To void double
>> acking (via I-Frame and later via RR) clear ack timer when sending
>> first pending I-Frame.
>>
>> Signed-off-by: Szymon Janc <[email protected]>
>> ---
>> ?net/bluetooth/l2cap_core.c | ? ?4 +++-
>> ?1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 5bb298d..8dece4e 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -1450,7 +1450,9 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
>>
>> ? ? ? ? ? ? ? ?if (bt_cb(skb)->retries == 1) {
>> ? ? ? ? ? ? ? ? ? ? ? ?chan->unacked_frames++;
>> - ? ? ? ? ? ? ? ? ? ? ? nsent++;
>> +
>> + ? ? ? ? ? ? ? ? ? ? ? if (!nsent++)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __clear_ack_timer(chan);
>> ? ? ? ? ? ? ? ?}
>>
>> ? ? ? ? ? ? ? ?chan->frames_sent++;
>
> If Luiz confirms the patches solve the problem I'm ok with them for now. Thanks.
>
> Best regards,
>
> --
> Ulisses Furquim
> ProFUSION embedded systems
> http://profusion.mobi
> Mobile: +55 19 9250 0942
> Skype: ulissesffs

Tested with test-server and test-client, both GET and PUT works.

Signed-off-by: Luiz Augusto von Dentz <[email protected]>

--
Luiz Augusto von Dentz

2012-02-07 15:47:32

by Luiz Augusto von Dentz

Subject: Re: [PATCH 1/2] Bluetooth: Fix possible missing I-Frame acknowledgement

Hi Szymon,

On Tue, Feb 7, 2012 at 4:43 PM, Szymon Janc <[email protected]> wrote:
> Make l2cap_ertm_send return number of pending I-Frames transmitted
> instead of all (pending + retransmitted) I-Frames transmitted.
>
> As only pending I-Frames are considered as acknowledgement, this could
> lead to situation when no ACK was sent in __l2cap_send_ack (if only
> already transmitted I-Frames were retransmitted).
>
> Signed-off-by: Szymon Janc <[email protected]>
> ---
> ?net/bluetooth/l2cap_core.c | ? ?6 +++---
> ?1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 09cd860..5bb298d 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1448,8 +1448,10 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
>
> ? ? ? ? ? ? ? ?chan->next_tx_seq = __next_seq(chan, chan->next_tx_seq);
>
> - ? ? ? ? ? ? ? if (bt_cb(skb)->retries == 1)
> + ? ? ? ? ? ? ? if (bt_cb(skb)->retries == 1) {
> ? ? ? ? ? ? ? ? ? ? ? ?chan->unacked_frames++;
> + ? ? ? ? ? ? ? ? ? ? ? nsent++;
> + ? ? ? ? ? ? ? }
>
> ? ? ? ? ? ? ? ?chan->frames_sent++;
>
> @@ -1457,8 +1459,6 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
> ? ? ? ? ? ? ? ? ? ? ? ?chan->tx_send_head = NULL;
> ? ? ? ? ? ? ? ?else
> ? ? ? ? ? ? ? ? ? ? ? ?chan->tx_send_head = skb_queue_next(&chan->tx_q, skb);
> -
> - ? ? ? ? ? ? ? nsent++;
> ? ? ? ?}
>
> ? ? ? ?return nsent;
> --
> on behalf of ST-Ericsson
>

Signed-off-by: Luiz Augusto von Dentz <[email protected]>

--
Luiz Augusto von Dentz

2012-02-07 15:26:41

by Ulisses Furquim

Subject: Re: [PATCH 2/2] Bluetooth: Fix double acking I-Frames when sending pending I-Frames

Hi Szymon,

On Tue, Feb 7, 2012 at 12:43 PM, Szymon Janc <[email protected]> wrote:
> Pending I-Frame(s) are considered as acknowledgement. To void double
> acking (via I-Frame and later via RR) clear ack timer when sending
> first pending I-Frame.
>
> Signed-off-by: Szymon Janc <[email protected]>
> ---
> ?net/bluetooth/l2cap_core.c | ? ?4 +++-
> ?1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 5bb298d..8dece4e 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1450,7 +1450,9 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)
>
> ? ? ? ? ? ? ? ?if (bt_cb(skb)->retries == 1) {
> ? ? ? ? ? ? ? ? ? ? ? ?chan->unacked_frames++;
> - ? ? ? ? ? ? ? ? ? ? ? nsent++;
> +
> + ? ? ? ? ? ? ? ? ? ? ? if (!nsent++)
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __clear_ack_timer(chan);
> ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ?chan->frames_sent++;

If Luiz confirms the patches solve the problem I'm ok with them for now. Thanks.

Best regards,

--
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs

2012-02-07 14:43:02

by Szymon Janc

Subject: [PATCH 2/2] Bluetooth: Fix double acking I-Frames when sending pending I-Frames

Pending I-Frame(s) are considered as acknowledgement. To void double
acking (via I-Frame and later via RR) clear ack timer when sending
first pending I-Frame.

Signed-off-by: Szymon Janc <[email protected]>
---
net/bluetooth/l2cap_core.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 5bb298d..8dece4e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1450,7 +1450,9 @@ static int l2cap_ertm_send(struct l2cap_chan *chan)

if (bt_cb(skb)->retries == 1) {
chan->unacked_frames++;
- nsent++;
+
+ if (!nsent++)
+ __clear_ack_timer(chan);
}

chan->frames_sent++;
--
on behalf of ST-Ericsson