tcp: fix delayed ack causing ack not to be sent after 3 packets. #530
Conversation
|
@Dirbaio can you pls give a couple of lines of information on what exactly is this "3 packet" thing ? I mean what is the conditions under which this can happen ? |
|
The delayed ack RFC specifies immediate ACKs must be sent for at least every 2 full fragments. The old implementation had 2 states, delayed ack timer "idle" and "running". ACKs are delayed when running, sent immediately when idle. First packet would change it to running, second would change it back to idle so that the immediate ack is sent. However if a 3rd packet arrived, it'd change the timer back to "running", so the ACK would be delayed when it shouldn't. The fix is to have 3 states: "idle", "running", and "immediate", so the 3rd packet leaves the timer in "immediate" state and the ACK is sent immediately. The bug is just a slight performance loss, ACKs were delayed when they shouldnt' in particular conditions, they were sent anyway so it still worked. |
|
@Dirbaio ok got it, thx for the explanation .. so before the fix, the code basically would send ack once every two packets like clock work. And now you are ensuring that the ACK is delayed only when moving from "no packets to first packet" and after that the ack is sent without delays (till the next "no packet" idle). |
Almost. Before, after receiving 3 packets in a row it would send a single, delayed ack. Now it sends a single, non-delayed ack. (it is possible for |
|
@Dirbaio got it, thx. I generally scan through the commits in the core of smoltcp just to know when I should update my private branch with the latest master. We have communicated before, I use smoltcp in a startup that I am trying to bootstrap, and of course I love smoltcp, its really easy to use! The reason I cant use smoltcp as is from master is because I have this use case where I have a really low memory environment where every MB of memory counts, but at the same time there can be many flows - lets say 200 - of which many of them might be idle, only maybe single digit ones are active. And if there are 200 odd idle flows and I supply smoltcp with a 64K buffer per flow, that means a dozen MB is eaten up already. So I had to come up with a mechanism by which I "reclaim" the 64K buffer from flows that are idle, and I know when exactly a flow gets packets, so just when it gets packets, I can "supply" smoltcp with a 64K buffer again. So that way the "in use" 64K buffers are only for the active flows. I dont know if there is a more elegant way of doing it - my 'hackish' changes are here - nextensio@043801a |
|
bors r+ |
|
@gopakumarce that's quite a niche use case! I'm not sure how (or whether) to accomodate that in smoltcp. The code assumes a fixed buffer size, dynamically resizing or swapping it is quite complicated! Perhaps making the buffer a trait, and allow users to supply their own impls..? |
|
@Dirbaio Yes, dynamic buffer sizes will be too complicated. Thankfully swapping it out was not as hard as I thought (diffs that I pointed to). I understand this might never be a mainline smoltcp requirement and hence ill just keep having to periodically rebase my branch with the mainline smoltcp |
|
Configuration problem: |
|
bors r+ |
No description provided.