From f3d76b39bfb7fe4325d8db3d58e037f3efe2cd4c Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 9 May 2024 12:41:08 +0800 Subject: [PATCH] make the initial packet size configurable (#4503) --- config.go | 11 ++++++ config_test.go | 20 ++++++++++ connection.go | 12 +++--- .../self/{dplpmtud_test.go => mtu_test.go} | 39 +++++++++++++++++-- interface.go | 6 +++ 5 files changed, 78 insertions(+), 10 deletions(-) rename integrationtests/self/{dplpmtud_test.go => mtu_test.go} (77%) diff --git a/config.go b/config.go index ee032e6e..d42bdc1c 100644 --- a/config.go +++ b/config.go @@ -39,6 +39,12 @@ func validateConfig(config *Config) error { if config.MaxConnectionReceiveWindow > quicvarint.Max { config.MaxConnectionReceiveWindow = quicvarint.Max } + if config.InitialPacketSize > 0 && config.InitialPacketSize < protocol.MinInitialPacketSize { + config.InitialPacketSize = protocol.MinInitialPacketSize + } + if config.InitialPacketSize > protocol.MaxPacketBufferSize { + config.InitialPacketSize = protocol.MaxPacketBufferSize + } // check that all QUIC versions are actually supported for _, v := range config.Versions { if !protocol.IsValidVersion(v) { @@ -94,6 +100,10 @@ func populateConfig(config *Config) *Config { } else if maxIncomingUniStreams < 0 { maxIncomingUniStreams = 0 } + initialPacketSize := config.InitialPacketSize + if initialPacketSize == 0 { + initialPacketSize = protocol.InitialPacketSize + } return &Config{ GetConfigForClient: config.GetConfigForClient, @@ -110,6 +120,7 @@ func populateConfig(config *Config) *Config { MaxIncomingUniStreams: maxIncomingUniStreams, TokenStore: config.TokenStore, EnableDatagrams: config.EnableDatagrams, + InitialPacketSize: initialPacketSize, DisablePathMTUDiscovery: config.DisablePathMTUDiscovery, Allow0RTT: config.Allow0RTT, Tracer: config.Tracer, diff --git a/config_test.go b/config_test.go index 8fdf2fae..6008a527 100644 --- a/config_test.go +++ b/config_test.go @@ -50,6 +50,24 @@ var _ = Describe("Config", func() { Expect(conf.MaxStreamReceiveWindow).To(BeEquivalentTo(uint64(quicvarint.Max))) Expect(conf.MaxConnectionReceiveWindow).To(BeEquivalentTo(uint64(quicvarint.Max))) }) + + It("increases too small packet sizes", func() { + conf := &Config{InitialPacketSize: 10} + Expect(validateConfig(conf)).To(Succeed()) + Expect(conf.InitialPacketSize).To(BeEquivalentTo(1200)) + }) + + It("clips too large packet sizes", func() { + conf := &Config{InitialPacketSize: protocol.MaxPacketBufferSize + 1} + Expect(validateConfig(conf)).To(Succeed()) + Expect(conf.InitialPacketSize).To(BeEquivalentTo(protocol.MaxPacketBufferSize)) + }) + + It("doesn't modify the InitialPacketSize if it is unset", func() { + conf := &Config{InitialPacketSize: 0} + Expect(validateConfig(conf)).To(Succeed()) + Expect(conf.InitialPacketSize).To(BeZero()) + }) }) configWithNonZeroNonFunctionFields := func() *Config { @@ -99,6 +117,8 @@ var _ = Describe("Config", func() { f.Set(reflect.ValueOf(true)) case "DisableVersionNegotiationPackets": f.Set(reflect.ValueOf(true)) + case "InitialPacketSize": + f.Set(reflect.ValueOf(uint16(1350))) case "DisablePathMTUDiscovery": f.Set(reflect.ValueOf(true)) case "Allow0RTT": diff --git a/connection.go b/connection.go index 6f415281..b88bc434 100644 --- a/connection.go +++ b/connection.go @@ -278,7 +278,7 @@ var newConnection = func( s.preSetup() s.sentPacketHandler, s.receivedPacketHandler = ackhandler.NewAckHandler( 0, - protocol.InitialPacketSize, + protocol.ByteCount(s.config.InitialPacketSize), s.rttStats, clientAddressValidated, s.conn.capabilities().ECN, @@ -286,8 +286,8 @@ var newConnection = func( s.tracer, s.logger, ) - s.mtuDiscoverer = newMTUDiscoverer(s.rttStats, protocol.InitialPacketSize, s.onMTUIncreased) - s.maxPayloadSizeEstimate.Store(uint32(estimateMaxPayloadSize(protocol.InitialPacketSize))) + s.mtuDiscoverer = newMTUDiscoverer(s.rttStats, protocol.ByteCount(s.config.InitialPacketSize), s.onMTUIncreased) + s.maxPayloadSizeEstimate.Store(uint32(estimateMaxPayloadSize(protocol.ByteCount(s.config.InitialPacketSize)))) params := &wire.TransportParameters{ InitialMaxStreamDataBidiLocal: protocol.ByteCount(s.config.InitialStreamReceiveWindow), InitialMaxStreamDataBidiRemote: protocol.ByteCount(s.config.InitialStreamReceiveWindow), @@ -389,7 +389,7 @@ var newClientConnection = func( s.preSetup() s.sentPacketHandler, s.receivedPacketHandler = ackhandler.NewAckHandler( initialPacketNumber, - protocol.InitialPacketSize, + protocol.ByteCount(s.config.InitialPacketSize), s.rttStats, false, // has no effect s.conn.capabilities().ECN, @@ -397,8 +397,8 @@ var newClientConnection = func( s.tracer, s.logger, ) - s.mtuDiscoverer = newMTUDiscoverer(s.rttStats, protocol.InitialPacketSize, s.onMTUIncreased) - s.maxPayloadSizeEstimate.Store(uint32(estimateMaxPayloadSize(protocol.InitialPacketSize))) + s.mtuDiscoverer = newMTUDiscoverer(s.rttStats, protocol.ByteCount(s.config.InitialPacketSize), s.onMTUIncreased) + s.maxPayloadSizeEstimate.Store(uint32(estimateMaxPayloadSize(protocol.ByteCount(s.config.InitialPacketSize)))) oneRTTStream := newCryptoStream() params := &wire.TransportParameters{ InitialMaxStreamDataBidiRemote: protocol.ByteCount(s.config.InitialStreamReceiveWindow), diff --git a/integrationtests/self/dplpmtud_test.go b/integrationtests/self/mtu_test.go similarity index 77% rename from integrationtests/self/dplpmtud_test.go rename to integrationtests/self/mtu_test.go index dd898b04..7336c5a3 100644 --- a/integrationtests/self/dplpmtud_test.go +++ b/integrationtests/self/mtu_test.go @@ -24,7 +24,11 @@ var _ = Describe("DPLPMTUD", func() { ln, err := quic.ListenAddr( "localhost:0", getTLSConfig(), - getQuicConfig(&quic.Config{DisablePathMTUDiscovery: true, EnableDatagrams: true}), + getQuicConfig(&quic.Config{ + InitialPacketSize: 1234, + DisablePathMTUDiscovery: true, + EnableDatagrams: true, + }), ) Expect(err).ToNot(HaveOccurred()) defer ln.Close() @@ -78,7 +82,10 @@ var _ = Describe("DPLPMTUD", func() { context.Background(), proxy.LocalAddr(), getTLSClientConfig(), - getQuicConfig(&quic.Config{EnableDatagrams: true}), + getQuicConfig(&quic.Config{ + InitialPacketSize: protocol.MinInitialPacketSize, + EnableDatagrams: true, + }), ) Expect(err).ToNot(HaveOccurred()) defer conn.CloseWithError(0, "") @@ -110,9 +117,33 @@ var _ = Describe("DPLPMTUD", func() { fmt.Fprintf(GinkgoWriter, "max server packet size: %d, MTU: %d\n", maxPacketSizeServer, mtu) Expect(maxPacketSizeClient).To(BeNumerically(">=", mtu-25)) const maxDiff = 40 // this includes the 21 bytes for the short header, 16 bytes for the encryption tag, and framing overhead - Expect(initialMaxDatagramSize).To(BeNumerically(">=", protocol.InitialPacketSize-maxDiff)) + Expect(initialMaxDatagramSize).To(BeNumerically(">=", protocol.MinInitialPacketSize-maxDiff)) Expect(finalMaxDatagramSize).To(BeNumerically(">=", maxPacketSizeClient-maxDiff)) // MTU discovery was disabled on the server side - Expect(maxPacketSizeServer).To(BeEquivalentTo(protocol.InitialPacketSize)) + Expect(maxPacketSizeServer).To(Equal(1234)) + }) + + It("uses the initial packet size", func() { + c, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0}) + Expect(err).ToNot(HaveOccurred()) + defer c.Close() + + cconn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0}) + Expect(err).ToNot(HaveOccurred()) + defer cconn.Close() + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + defer close(done) + quic.Dial(ctx, cconn, c.LocalAddr(), getTLSClientConfig(), getQuicConfig(&quic.Config{InitialPacketSize: 1337})) + }() + + b := make([]byte, 2000) + n, _, err := c.ReadFrom(b) + Expect(err).ToNot(HaveOccurred()) + Expect(n).To(Equal(1337)) + cancel() + Eventually(done).Should(BeClosed()) }) }) diff --git a/interface.go b/interface.go index ff0e9b8d..5837cb40 100644 --- a/interface.go +++ b/interface.go @@ -325,6 +325,12 @@ type Config struct { // If set to 0, then no keep alive is sent. Otherwise, the keep alive is sent on that period (or at most // every half of MaxIdleTimeout, whichever is smaller). KeepAlivePeriod time.Duration + // InitialPacketSize is the initial size of packets sent. + // It is usually not necessary to manually set this value, + // since Path MTU discovery very quickly finds the path's MTU. + // If set too high, the path might not support packets that large, leading to a timeout of the QUIC handshake. + // Values below 1200 are invalid. + InitialPacketSize uint16 // DisablePathMTUDiscovery disables Path MTU Discovery (RFC 8899). // This allows the sending of QUIC packets that fully utilize the available MTU of the path. // Path MTU discovery is only available on systems that allow setting of the Don't Fragment (DF) bit.