Hi, On Tue, Feb 7, 2012 at 9:02 AM, Szymon Janc <szymon.janc@xxxxxxxxx> wrote: > Hi, > >> >> But that means we would be clearing ack timer even for retransmit >> >> frames which Im not sure can be account as an ack? >> > >> > Right, only pending I-frame(s) is an acknowledgement. But that means >> > __l2cap_send_ack() is broken. >> >> Yes, it is broken because it was relying on l2cap_ertm_send() >> returning if it sent any i-frame with an ack. And that wasn't >> documented anywhere AFAIK and it's very easy for someone else to add a >> change that breaks this assumption. > > We can add comment for that. Yet, this is what I assumed when looked at that > code in first place, that it returns number of pending frames sent.. > >> >> > To fix that, I think l2cap_ertm_send should return number of pending I-frames >> > transmitted, not a total number of I-frames sent. >> >> Why? Have you checked if other part of the code relies on >> l2cap_ertm_send() returning number of I-frames sent? > > Yes, it looks like only __l2cap_send_ack() is interested if any frames were sent. > l2cap_chan_send is only interested if error occurred or not. Others just ignore > return value. > >> >> > I've quickly looked over the code and that change should be OK. >> > >> > What do you think? >> >> I've already answered in my other e-mail. I'd like to see us tracking >> what we already acked and then easily check if we need to send an ack >> or not. > > Well, if ack timer is running we have sth to ack. Isn't that enough? That should be the case. When ack timer is running we have something to ack. The point is answering if we still have pending acks to send. And that's being answered by the return of calling l2cap_ertm_send() which was wrong until now and I don't think it's a good idea. Marcel, any opinions on that? Luiz? Best 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
