From d1b0a97d84e0fa88851b5a065e23262afed10400 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 15 Aug 2023 11:11:36 -0400 Subject: [PATCH] quic: avoid sending 1-RTT frames in initial/handshake packets Restructure the send path a little to make it clear that 1-RTT frames go in 1-RTT packets. For golang/go#58547 Change-Id: Id4c2c86c8ccd350bf490f38a8bb01ad9bc2639ee Reviewed-on: https://go-review.googlesource.com/c/net/+/524035 Reviewed-by: Jonathan Amsterdam LUCI-TryBot-Result: Go LUCI --- internal/quic/conn_send.go | 40 +++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/internal/quic/conn_send.go b/internal/quic/conn_send.go index 6e6fbc5857..9d315fb392 100644 --- a/internal/quic/conn_send.go +++ b/internal/quic/conn_send.go @@ -224,14 +224,6 @@ func (c *Conn) appendFrames(now time.Time, space numberSpace, pnum packetNumber, // TODO: Add all the other frames we can send. - // HANDSHAKE_DONE - if c.handshakeConfirmed.shouldSendPTO(pto) { - if !c.w.appendHandshakeDoneFrame() { - return - } - c.handshakeConfirmed.setSent(pnum) - } - // CRYPTO c.crypto[space].dataToSend(pto, func(off, size int64) int64 { b, _ := c.w.appendCryptoFrame(off, int(size)) @@ -239,13 +231,6 @@ func (c *Conn) appendFrames(now time.Time, space numberSpace, pnum packetNumber, return int64(len(b)) }) - // NEW_CONNECTION_ID, RETIRE_CONNECTION_ID - if space == appDataSpace { - if !c.connIDState.appendFrames(&c.w, pnum, pto) { - return - } - } - // Test-only PING frames. if space == c.testSendPingSpace && c.testSendPing.shouldSendPTO(pto) { if !c.w.appendPingFrame() { @@ -254,11 +239,26 @@ func (c *Conn) appendFrames(now time.Time, space numberSpace, pnum packetNumber, c.testSendPing.setSent(pnum) } - // All stream-related frames. This should come last in the packet, - // so large amounts of STREAM data don't crowd out other frames - // we may need to send. - if !c.appendStreamFrames(&c.w, pnum, pto) { - return + if space == appDataSpace { + // HANDSHAKE_DONE + if c.handshakeConfirmed.shouldSendPTO(pto) { + if !c.w.appendHandshakeDoneFrame() { + return + } + c.handshakeConfirmed.setSent(pnum) + } + + // NEW_CONNECTION_ID, RETIRE_CONNECTION_ID + if !c.connIDState.appendFrames(&c.w, pnum, pto) { + return + } + + // All stream-related frames. This should come last in the packet, + // so large amounts of STREAM data don't crowd out other frames + // we may need to send. + if !c.appendStreamFrames(&c.w, pnum, pto) { + return + } } // If this is a PTO probe and we haven't added an ack-eliciting frame yet,