Hi Luiz, On Tue, Feb 7, 2012 at 8:21 AM, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > Hi Szymon, > > On Tue, Feb 7, 2012 at 10:08 AM, Szymon Janc <szymon.janc@xxxxxxxxx> wrote: >>> >>> Hmm, maybe we can clear ack timer only once if we check nsent in the >>> end of l2cap_ertm_send() instead of potentially call it several times? >>> Not sure if it's worth it or not, though. >> >> This is what __l2cap_send_ack is doing i.e. sends RR frame only if >> l2cap_ertm_send() returned 0 (or error). Would be good to have this consistent. >> >> So maybe just clear ack timer when incrementing nsent for the 1st time? >> >> if (!nsent++) >> __clear_ack_timer(chan); >> > > But that means we would be clearing ack timer even for retransmit > frames which Im not sure can be account as an ack? That's a good point. The retransmitted frames might have an ack, I think, but only ckecking nsent we won't know that for sure. Thus I do think the patch from Luiz is better but it's not the right fix IMO. We'd need to track the last ack sent and then we can check if we have pending acks. Seriously, I was wrong suggesting to Luiz that maybe a simple change in l2cap_ertm_send() to clear ack timer would solve the problem. This is not true and shows that our code is somewhat messy right now. Let's try to be more explicit and not rely on implicit conditions or behavior, please. Szymon, I've seen you sent patches to handle this before so if you (or Luiz) could send a patch to introduce tracking of last ack sent it'd be good. Then we can compute correctly if we have pending acks or not. Regards, -- Ulisses Furquim ProFUSION embedded systems http://profusion.mobi Mobile: +55 19 9250 0942 Skype: ulissesffs -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
