allow processing of multiple packets before handshake completion (#5451)

This is especially valuable:
* on the server side: if the ClientHello spans multiple packets (as it
  now does due to PQ cryptography)
* on the client side: if the server’s certificate span multiple packets
  as they do)

This avoids generating a flood of acknowledgments, since packets in the
Initial and in the Handshake packet number space are acknowledged
immediately.

diff --git a/connection.go b/connection.go
index adaad4a6d67e5ff0f601b2334beb8ba90a063bae..c72aee54dced67b47997b71632d2608115243c5b 100644 (file)
--- a/connection.go
+++ b/connection.go
@@ -1003,7 +1003,7 @@ func (c *Conn) handlePackets() (wasProcessed bool, _ error) {
        }
 
        var hasMorePackets bool
-       for i := 0; i < numPackets; i++ {
+       for i := range numPackets {
                if i > 0 {
                        c.receivedPacketMx.Lock()
                }
@@ -1022,11 +1022,10 @@ func (c *Conn) handlePackets() (wasProcessed bool, _ error) {
                if processed {
                        wasProcessed = true
                }
-               if !hasMorePackets {
+               if !c.handshakeComplete && (c.initialStream.HasData() || c.handshakeStream.HasData()) {
                        break
                }
-               // only process a single packet at a time before handshake completion
-               if !c.handshakeComplete {
+               if !hasMorePackets {
                        break
                }
        }
@@ -1043,8 +1042,7 @@ func (c *Conn) handleOnePacket(rp receivedPacket, datagramID qlog.DatagramID) (w
        c.sentPacketHandler.ReceivedBytes(rp.Size(), rp.rcvTime)
 
        if wire.IsVersionNegotiationPacket(rp.data) {
-               c.handleVersionNegotiationPacket(rp)
-               return false, nil
+               return false, c.handleVersionNegotiationPacket(rp)
        }
 
        var counter uint8
@@ -1556,7 +1554,7 @@ func (c *Conn) handleRetryPacket(hdr *wire.Header, data []byte, rcvTime monotime
        return true
 }
 
-func (c *Conn) handleVersionNegotiationPacket(p receivedPacket) {
+func (c *Conn) handleVersionNegotiationPacket(p receivedPacket) error {
        if c.perspective == protocol.PerspectiveServer || // servers never receive version negotiation packets
                c.receivedFirstPacket || c.versionNegotiated { // ignore delayed / duplicated version negotiation packets
                if c.qlogger != nil {
@@ -1566,7 +1564,7 @@ func (c *Conn) handleVersionNegotiationPacket(p receivedPacket) {
                                Trigger: qlog.PacketDropUnexpectedPacket,
                        })
                }
-               return
+               return nil
        }
 
        src, dest, supportedVersions, err := wire.ParseVersionNegotiationPacket(p.data)
@@ -1579,7 +1577,7 @@ func (c *Conn) handleVersionNegotiationPacket(p receivedPacket) {
                        })
                }
                c.logger.Debugf("Error parsing Version Negotiation packet: %s", err)
-               return
+               return nil
        }
 
        if slices.Contains(supportedVersions, c.version) {
@@ -1592,7 +1590,7 @@ func (c *Conn) handleVersionNegotiationPacket(p receivedPacket) {
                }
                // The Version Negotiation packet contains the version that we offered.
                // This might be a packet sent by an attacker, or it was corrupted.
-               return
+               return nil
        }
 
        c.logger.Infof("Received a Version Negotiation packet. Supported Versions: %s", supportedVersions)
@@ -1612,7 +1610,7 @@ func (c *Conn) handleVersionNegotiationPacket(p receivedPacket) {
                        Theirs: supportedVersions,
                })
                c.logger.Infof("No compatible QUIC version found.")
-               return
+               return nil
        }
        if c.qlogger != nil {
                c.qlogger.RecordEvent(qlog.VersionInformation{
@@ -1624,10 +1622,10 @@ func (c *Conn) handleVersionNegotiationPacket(p receivedPacket) {
 
        c.logger.Infof("Switching to QUIC version %s.", newVersion)
        nextPN, _ := c.sentPacketHandler.PeekPacketNumber(protocol.EncryptionInitial)
-       c.destroyImpl(&errCloseForRecreating{
+       return &errCloseForRecreating{
                nextPacketNumber: nextPN,
                nextVersion:      newVersion,
-       })
+       }
 }
 
 func (c *Conn) handleUnpackedLongHeaderPacket(

diff --git a/connection_test.go b/connection_test.go
index ed936b260ffc5591325aac723b2f1137da380836..08e280ac276f25686caee5b8a13c4bc2f90f096c 100644 (file)
--- a/connection_test.go
+++ b/connection_test.go
@@ -1542,22 +1542,12 @@ func TestConnection0RTTTransportParameters(t *testing.T) {
 }
 
 func TestConnectionReceivePrioritization(t *testing.T) {
-       t.Run("handshake complete", func(t *testing.T) {
-               events := testConnectionReceivePrioritization(t, true, 5)
-               require.Equal(t, []string{"unpack", "unpack", "unpack", "unpack", "unpack", "pack"}, events)
-       })
-
-       // before handshake completion, we trigger packing of a new packet every time we receive a packet
-       t.Run("handshake not complete", func(t *testing.T) {
-               events := testConnectionReceivePrioritization(t, false, 5)
-               require.Equal(t, []string{
-                       "unpack", "pack",
-                       "unpack", "pack",
-                       "unpack", "pack",
-                       "unpack", "pack",
-                       "unpack", "pack",
-               }, events)
-       })
+       for _, handshakeComplete := range []bool{true, false} {
+               t.Run(fmt.Sprintf("handshake complete: %t", handshakeComplete), func(t *testing.T) {
+                       events := testConnectionReceivePrioritization(t, handshakeComplete, 5)
+                       require.Equal(t, []string{"unpack", "unpack", "unpack", "unpack", "unpack", "pack"}, events)
+               })
+       }
 }
 
 func testConnectionReceivePrioritization(t *testing.T, handshakeComplete bool, numPackets int) []string {