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

Early retransmit code #934

Closed
siyengar opened this issue Nov 15, 2017 · 12 comments
Closed

Early retransmit code #934

siyengar opened this issue Nov 15, 2017 · 12 comments
Assignees
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.

Comments

@siyengar
Copy link

If we're in the early retransmit mode, a loss_time is set. If the timer is not yet fired, however we send a packet when the timer is armed, the largest_sent_packet changes. When the timer does fire before this new packet is acked, largest_sent_packet != largest_acked_packet, so delay_until_lost == infinite and none of the packets will be declared lost, however we would instead set a TLP alarm.

Is this intentional? Seems a bit weird.

@martinthomson
Copy link
Member

Note: Questions are better asked on the mailing list.

@siyengar
Copy link
Author

Ya it was a bit fuzzy on whether this was a bug in the draft, so I raised this as an issue.

@mnot mnot added the editorial An issue that does not affect the design of the protocol; does not require consensus. label Dec 12, 2017
@ianswett ianswett self-assigned this Mar 14, 2018
@siyengar
Copy link
Author

I think this is not an issue any more

@siyengar siyengar reopened this Jul 18, 2018
@siyengar
Copy link
Author

sorry this is so old that i forgot why i raised this, will discuss with @ianswett offline and then close properly.

@siyengar
Copy link
Author

More concrete example:

  1. Send packet 1, 2, 3, 4

  2. Ack 1,2,4.
    3 is lost
    largest_sent = 4
    largest_acked = 4

  3. loss_time is set now to send_time(3) + 5/4 rtt

  4. Send 5
    largest_sent = 5
    largest_acked = 4

  5. timer for 3 fires
    DetectLostPackets(4) is invoked

since largest_acked != largest_sent

delay_until_lost = infinite

so packet 3 is not lost in this round and needs to go through another round

@ianswett ianswett assigned siyengar and unassigned ianswett Aug 7, 2018
@ianswett
Copy link
Contributor

ianswett commented Aug 7, 2018

@siyengar You volunteered to write a PR, can you try fixing this issue you filed?

@siyengar
Copy link
Author

siyengar commented Aug 7, 2018

will do

@martinthomson
Copy link
Member

@siyengar, some time has passed. Do you still intend to do this?

@siyengar
Copy link
Author

@martinthomson put up #2021. Sorry about the delay

@janaiyengar
Copy link
Contributor

janaiyengar commented Nov 20, 2018 via email

@siyengar
Copy link
Author

@janaiyengar i think #1974 depends on how we add early retransmit timer back to that current PR. I think we might need something like this as well

@janaiyengar
Copy link
Contributor

@siyengar, does the merged #1974 resolve this? I believe it does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

No branches or pull requests

5 participants