Skip to content

Commit

Permalink
Queue Ping Probe on No Retransmissions Available (microsoft#356)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nibanks
nibanks authored May 4, 2020
1 parent 60716de commit 77654d9
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 74 deletions.
17 changes: 11 additions & 6 deletions src/core/crypto.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ QuicCryptoWriteFrames(
}

_IRQL_requires_max_(PASSIVE_LEVEL)
void
BOOLEAN
QuicCryptoOnLoss(
_In_ QUIC_CRYPTO* Crypto,
_In_ QUIC_SENT_FRAME_METADATA* FrameMetadata
Expand All @@ -743,7 +743,7 @@ QuicCryptoOnLoss(
//
// Already completely acknowledged.
//
return;
return FALSE;
} else if (Start < Crypto->UnAckedOffset) {
//
// The 'lost' range overlaps with UNA. Move Start forward.
Expand All @@ -767,7 +767,7 @@ QuicCryptoOnLoss(
//
// The SACK fully covers the whole 'lost' range.
//
return;
return FALSE;

} else {
//
Expand Down Expand Up @@ -825,12 +825,17 @@ QuicCryptoOnLoss(
Crypto->InRecovery = TRUE;
}

QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_CRYPTO);
BOOLEAN DataQueued =
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_CRYPTO);

QuicCryptoDumpSendState(Crypto);

return DataQueued;
}

return FALSE;
}

_IRQL_requires_max_(PASSIVE_LEVEL)
Expand Down
5 changes: 3 additions & 2 deletions src/core/crypto.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,11 @@ QuicCryptoWriteFrames(
);

//
// Called when a crypto frame is inferred to be lost.
// Called when a crypto frame is inferred to be lost. Returns TRUE if data is
// queued to be sent.
//
_IRQL_requires_max_(PASSIVE_LEVEL)
void
BOOLEAN
QuicCryptoOnLoss(
_In_ QUIC_CRYPTO* Crypto,
_In_ QUIC_SENT_FRAME_METADATA* FrameMetadata
Expand Down
119 changes: 68 additions & 51 deletions src/core/loss_detection.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
#include "precomp.h"

_IRQL_requires_max_(PASSIVE_LEVEL)
void
BOOLEAN
QuicLossDetectionRetransmitFrames(
_In_ QUIC_LOSS_DETECTION* LossDetection,
_In_ QUIC_SENT_PACKET_METADATA* Packet
Expand Down Expand Up @@ -513,45 +513,50 @@ QuicLossDetectionOnPacketAcknowledged(

//
// Marks all the frames in the packet that can be retransmitted as needing to be
// retransmitted.
// retransmitted. Returns TRUE if some new data was queued up to be sent.
//
_IRQL_requires_max_(PASSIVE_LEVEL)
void
BOOLEAN
QuicLossDetectionRetransmitFrames(
_In_ QUIC_LOSS_DETECTION* LossDetection,
_In_ QUIC_SENT_PACKET_METADATA* Packet
)
{
QUIC_CONNECTION* Connection = QuicLossDetectionGetConnection(LossDetection);
BOOLEAN NewDataQueued = FALSE;

for (uint8_t i = 0; i < Packet->FrameCount; i++) {
switch (Packet->Frames[i].Type) {
case QUIC_FRAME_PING:
if (!Packet->Flags.IsPMTUD) {
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_PING);
NewDataQueued |=
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_PING);
}
break;

case QUIC_FRAME_RESET_STREAM:
QuicSendSetStreamSendFlag(
&Connection->Send,
Packet->Frames[i].RESET_STREAM.Stream,
QUIC_STREAM_SEND_FLAG_SEND_ABORT);
NewDataQueued |=
QuicSendSetStreamSendFlag(
&Connection->Send,
Packet->Frames[i].RESET_STREAM.Stream,
QUIC_STREAM_SEND_FLAG_SEND_ABORT);
break;

case QUIC_FRAME_STOP_SENDING:
QuicSendSetStreamSendFlag(
&Connection->Send,
Packet->Frames[i].STOP_SENDING.Stream,
QUIC_STREAM_SEND_FLAG_RECV_ABORT);
NewDataQueued |=
QuicSendSetStreamSendFlag(
&Connection->Send,
Packet->Frames[i].STOP_SENDING.Stream,
QUIC_STREAM_SEND_FLAG_RECV_ABORT);
break;

case QUIC_FRAME_CRYPTO:
QuicCryptoOnLoss(
&Connection->Crypto,
&Packet->Frames[i]);
NewDataQueued |=
QuicCryptoOnLoss(
&Connection->Crypto,
&Packet->Frames[i]);
break;

case QUIC_FRAME_STREAM:
Expand All @@ -562,41 +567,47 @@ QuicLossDetectionRetransmitFrames(
case QUIC_FRAME_STREAM_5:
case QUIC_FRAME_STREAM_6:
case QUIC_FRAME_STREAM_7:
QuicStreamOnLoss(
Packet->Frames[i].STREAM.Stream,
&Packet->Frames[i]);
NewDataQueued |=
QuicStreamOnLoss(
Packet->Frames[i].STREAM.Stream,
&Packet->Frames[i]);
break;

case QUIC_FRAME_MAX_DATA:
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_MAX_DATA);
NewDataQueued |=
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_MAX_DATA);
break;

case QUIC_FRAME_MAX_STREAM_DATA:
QuicSendSetStreamSendFlag(
&Connection->Send,
Packet->Frames[i].MAX_STREAM_DATA.Stream,
QUIC_STREAM_SEND_FLAG_MAX_DATA);
NewDataQueued |=
QuicSendSetStreamSendFlag(
&Connection->Send,
Packet->Frames[i].MAX_STREAM_DATA.Stream,
QUIC_STREAM_SEND_FLAG_MAX_DATA);
break;

case QUIC_FRAME_MAX_STREAMS:
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_MAX_STREAMS_BIDI);
NewDataQueued |=
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_MAX_STREAMS_BIDI);
break;

case QUIC_FRAME_MAX_STREAMS_1:
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_MAX_STREAMS_UNI);
NewDataQueued |=
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_MAX_STREAMS_UNI);
break;

case QUIC_FRAME_STREAM_DATA_BLOCKED:
QuicSendSetStreamSendFlag(
&Connection->Send,
Packet->Frames[i].STREAM_DATA_BLOCKED.Stream,
QUIC_STREAM_SEND_FLAG_DATA_BLOCKED);
NewDataQueued |=
QuicSendSetStreamSendFlag(
&Connection->Send,
Packet->Frames[i].STREAM_DATA_BLOCKED.Stream,
QUIC_STREAM_SEND_FLAG_DATA_BLOCKED);
break;

case QUIC_FRAME_NEW_CONNECTION_ID: {
Expand All @@ -610,9 +621,10 @@ QuicLossDetectionRetransmitFrames(
if (SourceCid != NULL &&
!SourceCid->CID.Acknowledged) {
SourceCid->CID.NeedsToSend = TRUE;
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_NEW_CONNECTION_ID);
NewDataQueued |=
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_NEW_CONNECTION_ID);
}
break;
}
Expand All @@ -626,9 +638,10 @@ QuicLossDetectionRetransmitFrames(
if (DestCid != NULL) {
QUIC_DBG_ASSERT(DestCid->CID.Retired);
DestCid->CID.NeedsToSend = TRUE;
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_RETIRE_CONNECTION_ID);
NewDataQueued |=
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_RETIRE_CONNECTION_ID);
}
break;
}
Expand All @@ -637,20 +650,24 @@ QuicLossDetectionRetransmitFrames(
QUIC_PATH* Path = QuicConnGetPathByID(Connection, Packet->PathId);
if (Path != NULL && !Path->IsPeerValidated) {
Path->SendChallenge = TRUE;
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_PATH_CHALLENGE);
NewDataQueued |=
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_PATH_CHALLENGE);
}
break;
}

case QUIC_FRAME_HANDSHAKE_DONE:
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_HANDSHAKE_DONE);
NewDataQueued |=
QuicSendSetSendFlag(
&Connection->Send,
QUIC_CONN_SEND_FLAG_HANDSHAKE_DONE);
break;
}
}

return NewDataQueued;
}

//
Expand Down Expand Up @@ -1324,8 +1341,8 @@ QuicLossDetectionScheduleProbe(
Packet->PacketNumber,
QuicPacketTraceType(Packet),
QUIC_TRACE_PACKET_LOSS_PROBE);
QuicLossDetectionRetransmitFrames(LossDetection, Packet);
if (--NumPackets == 0) {
if (QuicLossDetectionRetransmitFrames(LossDetection, Packet) &&
--NumPackets == 0) {
return;
}
}
Expand Down
10 changes: 7 additions & 3 deletions src/core/send.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ QuicSendValidate(
#endif

_IRQL_requires_max_(DISPATCH_LEVEL)
void
BOOLEAN
QuicSendSetSendFlag(
_In_ QUIC_SEND* Send,
_In_ uint32_t SendFlags
Expand Down Expand Up @@ -245,6 +245,8 @@ QuicSendSetSendFlag(
}

QuicSendValidate(Send);

return CanSetFlag;
}

_IRQL_requires_max_(DISPATCH_LEVEL)
Expand Down Expand Up @@ -301,7 +303,7 @@ QuicSendUpdateAckState(
}

_IRQL_requires_max_(PASSIVE_LEVEL)
void
BOOLEAN
QuicSendSetStreamSendFlag(
_In_ QUIC_SEND* Send,
_In_ QUIC_STREAM* Stream,
Expand All @@ -313,7 +315,7 @@ QuicSendSetStreamSendFlag(
//
// Ignore all frames if the connection is closed.
//
return;
return FALSE;
}

//
Expand Down Expand Up @@ -358,6 +360,8 @@ QuicSendSetStreamSendFlag(
}
Stream->SendFlags |= SendFlags;
}

return SendFlags != 0;
}

_IRQL_requires_max_(PASSIVE_LEVEL)
Expand Down
8 changes: 4 additions & 4 deletions src/core/send.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,10 @@ QuicSendProcessDelayedAckTimer(

//
// Indicates the connection has a given QUIC_CONN_SEND_FLAG_* that is ready
// to be sent.
// to be sent. Returns TRUE if the flag is (or already was) queued.
//
_IRQL_requires_max_(DISPATCH_LEVEL)
void
BOOLEAN
QuicSendSetSendFlag(
_In_ QUIC_SEND* Send,
_In_ uint32_t SendFlag
Expand All @@ -367,10 +367,10 @@ QuicSendUpdateAckState(

//
// Indicates the stream has a given QUIC_STREAM_SEND_FLAG_* that is ready
// to be sent.
// to be sent. Returns TRUE if the flag is (or already was) queued.
//
_IRQL_requires_max_(PASSIVE_LEVEL)
void
BOOLEAN
QuicSendSetStreamSendFlag(
_In_ QUIC_SEND* Send,
_In_ QUIC_STREAM* Stream,
Expand Down
5 changes: 3 additions & 2 deletions src/core/stream.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -714,10 +714,11 @@ QuicStreamSendWrite(
);

//
// Called when a stream frame is inferred to be lost.
// Called when a stream frame is inferred to be lost. Returns TRUE if data is
// queued to be sent.
//
_IRQL_requires_max_(PASSIVE_LEVEL)
void
BOOLEAN
QuicStreamOnLoss(
_In_ QUIC_STREAM* Stream,
_In_ QUIC_SENT_FRAME_METADATA* FrameMetadata
Expand Down
Loading

0 comments on commit 77654d9

Please sign in to comment.