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

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

[Szymon Janc]

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 <szymon.janc@tieto.com>
---
 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

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

[Szymon Janc]

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 <szymon.janc@tieto.com>
---
 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

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

[Ulisses Furquim]

Hi Szymon,

On Tue, Feb 7, 2012 at 12:43 PM, Szymon Janc <szymon.janc@tieto.com> 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 <szymon.janc@tieto.com>
> ---
>  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

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

[Luiz Augusto von Dentz]

Hi Szymon,

On Tue, Feb 7, 2012 at 4:43 PM, Szymon Janc <szymon.janc@tieto.com> 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 <szymon.janc@tieto.com>
> ---
>  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 <luiz.von.dentz@intel.com>

-- 
Luiz Augusto von Dentz

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

[Luiz Augusto von Dentz]

Hi Szymon,

On Tue, Feb 7, 2012 at 5:26 PM, Ulisses Furquim <ulisses@profusion.mobi> wrote:
> Hi Szymon,
>
> On Tue, Feb 7, 2012 at 12:43 PM, Szymon Janc <szymon.janc@tieto.com> 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 <szymon.janc@tieto.com>
>> ---
>>  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 <luiz.von.dentz@intel.com>

-- 
Luiz Augusto von Dentz

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

[Gustavo Padovan]

Hi Szymon,

* Szymon Janc <szymon.janc@tieto.com> [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 <szymon.janc@tieto.com>
> ---
>  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 <padovan@profusion.mobi>

	Gustavo

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

[Marcel Holtmann]

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 <szymon.janc@tieto.com>
> ---
>  net/bluetooth/l2cap_core.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

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

[Marcel Holtmann]

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 <szymon.janc@tieto.com>
> ---
>  net/bluetooth/l2cap_core.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

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

[Johan Hedberg]

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 <szymon.janc@tieto.com>
> ---
>  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