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

PTO MUST send new data or retransmit data if possible #3057

Merged
merged 7 commits into from Oct 30, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 14 additions & 14 deletions draft-ietf-quic-recovery.md
Expand Up @@ -495,11 +495,9 @@ a PATH_RESPONSE to seed initial_rtt for a new path, but the delay SHOULD NOT
be considered an RTT sample.

Until the server has validated the client's address on the path, the amount of
data it can send is limited, as specified in Section 8.1 of {{QUIC-TRANSPORT}}.
Data at Initial encryption MUST be retransmitted before Handshake data and
data at Handshake encryption MUST be retransmitted before any ApplicationData
data. If no data can be sent, then the PTO alarm MUST NOT be armed until
data has been received from the client.
data it can send is limited to three times the amount of data received,
as specified in Section 8.1 of {{QUIC-TRANSPORT}}. If no data can be sent,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to allow PINGs. "... and no other ack-eliciting packet is sent".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting we always arm the PTO and if we've hit the amplification factor, the server should send a PING-only packet? That actually sounds easier than not arming it and prompts the client to send a padded Initial ACK, but I'm not sure if that's what you're suggesting?

Copy link
Contributor Author

@ianswett ianswett Oct 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created #3161 to discuss whether we want to make a design change around the server's response to reaching the amplification limit, FYI.

then the PTO alarm MUST NOT be armed.

Since the server could be blocked until more packets are received from the
client, it is the client's responsibility to send packets to unblock the server
Expand All @@ -526,15 +524,17 @@ as a probe, unless there is no data available to send. An endpoint MAY send up
to two full-sized datagrams containing ack-eliciting packets, to avoid an
expensive consecutive PTO expiration due to a single lost datagram.

It is possible that the sender has no new or previously-sent data to send. As
an example, consider the following sequence of events: new application data is
sent in a STREAM frame, deemed lost, then retransmitted in a new packet, and
then the original transmission is acknowledged. In the absence of any new
application data, a PTO timer expiration now would find the sender with no new
or previously-sent data to send.

When there is no data to send, the sender SHOULD send a PING or other
ack-eliciting frame in a single packet, re-arming the PTO timer.
When the PTO timer expires, and there is new or previously sent unacknowledged
data, it MUST be sent. Data that was previously sent with Initial encryption
MUST be sent before Handshake data and data previously sent at Handshake
encryption MUST be sent before any ApplicationData data.

It is possible the sender has no new or previously-sent data to send.
As an example, consider the following sequence of events: new application data
is sent in a STREAM frame, deemed lost, then retransmitted in a new packet,
and then the original transmission is acknowledged. When there is no data to
send, the sender SHOULD send a PING or other ack-eliciting frame in a single
packet, re-arming the PTO timer.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get this example. In this example, you have sent something, it was received, end of story.

The new text above says that you can't arm PTO if you have nothing to send (MUST NOT even). Which did you mean?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, the second packet is ack-eliciting and has not been acknowledged, eventually leading to a probe timeout. However, there is no unsent or sent-but-unacknowledged ack-eliciting data available to send, so a PING is fabricated instead.

Perhaps "there is new or previously sent data" should be more specific?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this kind of situation only happens when client sent Initial and received its ACK but either sever Initial or Handshake was lost. Client then literally has nothing to send. And forced to send ack-eliciting frame (and controversially, draft-23 does not allow PING here).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this example, the point is that in order to remove the retransmission from byte in flight, you SHOULD send a PING and when it's acknowledged, you'll know whether the retransmission was lost or not. Ideally it'll be ACKed on its own before the PTO fires.

The above text is about the server's amplification factor and not arming the alarm when "If no data can be sent"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not working for me.

I get the need for the client to send extra Initial packets again in the case where it has nothing to say, but that should be a special provision.

If p1 is acknowledged and p2 contained nothing new, then let it lie. You don't need to know whether it was lost or not.

I get that this packet will stay outstanding unless you have something new to say, but you are only sending a PING to deal with an accounting discrepancy. That only becomes relevant when you go to send the next packet. Is there any way that this sort of blip can end up consuming the entire congestion window?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the problem this was intended to address was exactly that accounting discrepancy. There are two ways to handle outstanding packets if we want to give up on them: call them lost and reduce the window (see text below, maybe that should be part of this paragraph), or send a PING to elicit an ACK from the receiver. The text used to recommend the calling them lost IIRC, but I don't see a clear winner here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this statement?

Data that was previously sent with Initial encryption
MUST be sent before Handshake data and data previously sent at Handshake
MUST be sent before any ApplicationData data.

Doesn't this conflict with the "PTO per packet number space" PR? If I'm implementing that PR most literally, I should PTO only the data from the space that expired. For example, I can have the 0RTT/1RTT PTO timer running first with some handshake data also in flight, and I should send just 0RTT/1RTT.

If I'm coalescing, I'm not sure why this would matter.

As for "MUST send new/unacked data", is that necessary? Why is not OK to send a PING instead to reduce bytes in flight? Perhaps I'm expecting new data soon and want to preserve the window space? I'd prefer if this were a SHOULD.

Copy link
Contributor Author

@ianswett ianswett Oct 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but this is existing text and I didn't want to tie this PR to the PTO per PN space PR.

One of the reasons I'd like to get this change in soon is it's going to cause conflicts with the multiple PN spaces PR, and the other PR is more substantive.

The original issue(#3056) was that there was no MUST to retransmit the client Initial, and if the server doesn't create connection state until receiving a client Initial, the handshake fails.

That being said, the client sending a full sized ack eliciting packet that's not the ClientHello seems odd. And if the server isn't going to create state, it seems like it should send a Retry to do path validation.


Alternatively, instead of sending an ack-eliciting packet, the sender MAY mark
any packets still in flight as lost. Doing so avoids sending an additional
Expand Down