Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid Assumptions in packet_sent triggers #13

Closed
nibanks opened this issue Jul 23, 2019 · 1 comment
Closed

Invalid Assumptions in packet_sent triggers #13

nibanks opened this issue Jul 23, 2019 · 1 comment

Comments

@nibanks
Copy link
Member

nibanks commented Jul 23, 2019

The packet_sent's triggers field makes the assumptions that a packet is sent because a previous packet is being retransmitted. For instance, in winquic a connection has a queue/set of what data needs to be sent. When data is (suspected) lost, the data in the packet is added back to the queue. Similarly for PTO, we mark an outstanding packet as lost, and if we don't have any, queue a PING frame.

The send logic just grabs data from the queue/set and builds up packets to be sent. There is no direction relationship between different packets.

So, IMO, triggers are the reason data is queued to be sent, not actually sent. What is actually sent will depend on the entire state of the send queue at the time the send logic actually executes.

For example, assume you have two outstanding packets that end up getting marked as lost:

  PktNum=1 { STREAM ID=1, Offset=0, Length=100 }
  PktNum=2 { STREAM ID=1, Offset=100, Length=100 }

Both are marked as lost. Around the same time, the app queues another 100 bytes on stream 1 to be sent. Then another packet ends up getting sent:

  PktNum=55 { STREAM ID=1, Offset=0, Length=300 }
@nibanks
Copy link
Member Author

nibanks commented Jul 23, 2019

As a follow up, I believe the packet_lost and packet_acknowledged events should be in the transport section. Also, the packet_retransmit should be removed.

@rmarx rmarx closed this as completed in 4f30c9c Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant