Skip to content

Commit

Permalink
Move max UDP payload size to transport
Browse files Browse the repository at this point in the history
  • Loading branch information
chungthuang committed Jul 12, 2024
1 parent d136868 commit 3e1fff0
Show file tree
Hide file tree
Showing 11 changed files with 77 additions and 59 deletions.
36 changes: 20 additions & 16 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ type client struct {
tlsConf *tls.Config
config *Config

connIDGenerator ConnectionIDGenerator
srcConnID protocol.ConnectionID
destConnID protocol.ConnectionID
connIDGenerator ConnectionIDGenerator
maxUDPPayloadSize protocol.ByteCount
srcConnID protocol.ConnectionID
destConnID protocol.ConnectionID

initialPacketNumber protocol.PacketNumber
hasNegotiatedVersion bool
Expand Down Expand Up @@ -142,8 +143,9 @@ func dial(
config *Config,
onClose func(),
use0RTT bool,
maxUDPPayloadSize protocol.ByteCount,
) (quicConn, error) {
c, err := newClient(conn, connIDGenerator, config, tlsConf, onClose, use0RTT)
c, err := newClient(conn, connIDGenerator, config, tlsConf, onClose, use0RTT, maxUDPPayloadSize)
if err != nil {
return nil, err
}
Expand All @@ -162,7 +164,7 @@ func dial(
return c.conn, nil
}

func newClient(sendConn sendConn, connIDGenerator ConnectionIDGenerator, config *Config, tlsConf *tls.Config, onClose func(), use0RTT bool) (*client, error) {
func newClient(sendConn sendConn, connIDGenerator ConnectionIDGenerator, config *Config, tlsConf *tls.Config, onClose func(), use0RTT bool, maxUDPPayloadSize protocol.ByteCount) (*client, error) {
srcConnID, err := connIDGenerator.GenerateConnectionID()
if err != nil {
return nil, err
Expand All @@ -172,17 +174,18 @@ func newClient(sendConn sendConn, connIDGenerator ConnectionIDGenerator, config
return nil, err
}
c := &client{
connIDGenerator: connIDGenerator,
srcConnID: srcConnID,
destConnID: destConnID,
sendConn: sendConn,
use0RTT: use0RTT,
onClose: onClose,
tlsConf: tlsConf,
config: config,
version: config.Versions[0],
handshakeChan: make(chan struct{}),
logger: utils.DefaultLogger.WithPrefix("client"),
connIDGenerator: connIDGenerator,
srcConnID: srcConnID,
destConnID: destConnID,
sendConn: sendConn,
use0RTT: use0RTT,
onClose: onClose,
tlsConf: tlsConf,
config: config,
maxUDPPayloadSize: maxUDPPayloadSize,
version: config.Versions[0],
handshakeChan: make(chan struct{}),
logger: utils.DefaultLogger.WithPrefix("client"),
}
return c, nil
}
Expand All @@ -205,6 +208,7 @@ func (c *client) dial(ctx context.Context) error {
c.tracer,
c.logger,
c.version,
c.maxUDPPayloadSize,
)
c.packetHandlers.Add(c.srcConnID, c.conn)

Expand Down
12 changes: 9 additions & 3 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ var _ = Describe("Client", func() {
tracer *logging.ConnectionTracer,
logger utils.Logger,
v protocol.Version,
maxUDPPayloadSize protocol.ByteCount,
) quicConn
)

Expand Down Expand Up @@ -126,6 +127,7 @@ var _ = Describe("Client", func() {
_ *logging.ConnectionTracer,
_ utils.Logger,
_ protocol.Version,
_ protocol.ByteCount,
) quicConn {
Expect(enable0RTT).To(BeFalse())
conn := NewMockQUICConn(mockCtrl)
Expand All @@ -135,7 +137,7 @@ var _ = Describe("Client", func() {
conn.EXPECT().HandshakeComplete().Return(c)
return conn
}
cl, err := newClient(packetConn, &protocol.DefaultConnectionIDGenerator{}, populateConfig(config), tlsConf, nil, false)
cl, err := newClient(packetConn, &protocol.DefaultConnectionIDGenerator{}, populateConfig(config), tlsConf, nil, false, protocol.DefaultMaxUDPPayloadSize)
Expect(err).ToNot(HaveOccurred())
cl.packetHandlers = manager
Expect(cl).ToNot(BeNil())
Expand Down Expand Up @@ -163,6 +165,7 @@ var _ = Describe("Client", func() {
_ *logging.ConnectionTracer,
_ utils.Logger,
_ protocol.Version,
_ protocol.ByteCount,
) quicConn {
Expect(enable0RTT).To(BeTrue())
conn := NewMockQUICConn(mockCtrl)
Expand All @@ -172,7 +175,7 @@ var _ = Describe("Client", func() {
return conn
}

cl, err := newClient(packetConn, &protocol.DefaultConnectionIDGenerator{}, populateConfig(config), tlsConf, nil, true)
cl, err := newClient(packetConn, &protocol.DefaultConnectionIDGenerator{}, populateConfig(config), tlsConf, nil, true, protocol.DefaultMaxUDPPayloadSize)
Expect(err).ToNot(HaveOccurred())
cl.packetHandlers = manager
Expect(cl).ToNot(BeNil())
Expand Down Expand Up @@ -200,6 +203,7 @@ var _ = Describe("Client", func() {
_ *logging.ConnectionTracer,
_ utils.Logger,
_ protocol.Version,
_ protocol.ByteCount,
) quicConn {
conn := NewMockQUICConn(mockCtrl)
conn.EXPECT().run().Return(testErr)
Expand All @@ -208,7 +212,7 @@ var _ = Describe("Client", func() {
return conn
}
var closed bool
cl, err := newClient(packetConn, &protocol.DefaultConnectionIDGenerator{}, populateConfig(config), tlsConf, func() { closed = true }, true)
cl, err := newClient(packetConn, &protocol.DefaultConnectionIDGenerator{}, populateConfig(config), tlsConf, func() { closed = true }, true, protocol.DefaultMaxUDPPayloadSize)
Expect(err).ToNot(HaveOccurred())
cl.packetHandlers = manager
Expect(cl).ToNot(BeNil())
Expand Down Expand Up @@ -285,6 +289,7 @@ var _ = Describe("Client", func() {
_ *logging.ConnectionTracer,
_ utils.Logger,
versionP protocol.Version,
_ protocol.ByteCount,
) quicConn {
version = versionP
conf = configP
Expand Down Expand Up @@ -328,6 +333,7 @@ var _ = Describe("Client", func() {
_ *logging.ConnectionTracer,
_ utils.Logger,
versionP protocol.Version,
_ protocol.ByteCount,
) quicConn {
conn := NewMockQUICConn(mockCtrl)
conn.EXPECT().HandshakeComplete().Return(make(chan struct{}))
Expand Down
11 changes: 0 additions & 11 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,6 @@ func validateConfig(config *Config) error {
if config.InitialPacketSize > protocol.MaxPacketBufferSize {
config.InitialPacketSize = protocol.MaxPacketBufferSize
}
if config.MaxUDPPayloadSize > 0 && config.MaxUDPPayloadSize < protocol.MinInitialPacketSize {
config.MaxUDPPayloadSize = protocol.MinInitialPacketSize
}
if config.MaxUDPPayloadSize > protocol.MaxPacketBufferSize {
config.MaxUDPPayloadSize = protocol.MaxPacketBufferSize
}
// check that all QUIC versions are actually supported
for _, v := range config.Versions {
if !protocol.IsValidVersion(v) {
Expand Down Expand Up @@ -110,10 +104,6 @@ func populateConfig(config *Config) *Config {
if initialPacketSize == 0 {
initialPacketSize = protocol.InitialPacketSize
}
maxUDPPayloadSize := config.MaxUDPPayloadSize
if maxUDPPayloadSize == 0 {
maxUDPPayloadSize = protocol.DefaultMaxUDPPayloadSize
}

return &Config{
GetConfigForClient: config.GetConfigForClient,
Expand All @@ -131,7 +121,6 @@ func populateConfig(config *Config) *Config {
TokenStore: config.TokenStore,
EnableDatagrams: config.EnableDatagrams,
InitialPacketSize: initialPacketSize,
MaxUDPPayloadSize: maxUDPPayloadSize,
DisablePathMTUDiscovery: config.DisablePathMTUDiscovery,
Allow0RTT: config.Allow0RTT,
Tracer: config.Tracer,
Expand Down
20 changes: 0 additions & 20 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,24 +68,6 @@ var _ = Describe("Config", func() {
Expect(validateConfig(conf)).To(Succeed())
Expect(conf.InitialPacketSize).To(BeZero())
})

It("increases too small UDP payload sizes", func() {
conf := &Config{MaxUDPPayloadSize: 10}
Expect(validateConfig(conf)).To(Succeed())
Expect(conf.MaxUDPPayloadSize).To(BeEquivalentTo(1200))
})

It("clips too large UDP payload sizes", func() {
conf := &Config{MaxUDPPayloadSize: protocol.MaxPacketBufferSize + 1}
Expect(validateConfig(conf)).To(Succeed())
Expect(conf.MaxUDPPayloadSize).To(BeEquivalentTo(protocol.MaxPacketBufferSize))
})

It("doesn't modify the MaxUDPPayloadSize if it is unset", func() {
conf := &Config{MaxUDPPayloadSize: 0}
Expect(validateConfig(conf)).To(Succeed())
Expect(conf.MaxUDPPayloadSize).To(BeZero())
})
})

configWithNonZeroNonFunctionFields := func() *Config {
Expand Down Expand Up @@ -137,8 +119,6 @@ var _ = Describe("Config", func() {
f.Set(reflect.ValueOf(true))
case "InitialPacketSize":
f.Set(reflect.ValueOf(uint16(1350)))
case "MaxUDPPayloadSize":
f.Set(reflect.ValueOf(uint16(1400)))
case "DisablePathMTUDiscovery":
f.Set(reflect.ValueOf(true))
case "Allow0RTT":
Expand Down
6 changes: 4 additions & 2 deletions connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ var newConnection = func(
tracer *logging.ConnectionTracer,
logger utils.Logger,
v protocol.Version,
maxUDPPayloadSize protocol.ByteCount,
) quicConn {
s := &connection{
ctx: ctx,
Expand Down Expand Up @@ -298,7 +299,7 @@ var newConnection = func(
MaxUniStreamNum: protocol.StreamNum(s.config.MaxIncomingUniStreams),
MaxAckDelay: protocol.MaxAckDelayInclGranularity,
AckDelayExponent: protocol.AckDelayExponent,
MaxUDPPayloadSize: protocol.ByteCount(s.config.MaxUDPPayloadSize),
MaxUDPPayloadSize: maxUDPPayloadSize,
DisableActiveMigration: true,
StatelessResetToken: &statelessResetToken,
OriginalDestinationConnectionID: origDestConnID,
Expand Down Expand Up @@ -354,6 +355,7 @@ var newClientConnection = func(
tracer *logging.ConnectionTracer,
logger utils.Logger,
v protocol.Version,
maxUDPPayloadSize protocol.ByteCount,
) quicConn {
s := &connection{
conn: conn,
Expand Down Expand Up @@ -408,7 +410,7 @@ var newClientConnection = func(
MaxBidiStreamNum: protocol.StreamNum(s.config.MaxIncomingStreams),
MaxUniStreamNum: protocol.StreamNum(s.config.MaxIncomingUniStreams),
MaxAckDelay: protocol.MaxAckDelayInclGranularity,
MaxUDPPayloadSize: protocol.ByteCount(s.config.MaxUDPPayloadSize),
MaxUDPPayloadSize: maxUDPPayloadSize,
AckDelayExponent: protocol.AckDelayExponent,
DisableActiveMigration: true,
// For interoperability with quic-go versions before May 2023, this value must be set to a value
Expand Down
2 changes: 2 additions & 0 deletions connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ var _ = Describe("Connection", func() {
tr,
utils.DefaultLogger,
protocol.Version1,
protocol.DefaultMaxUDPPayloadSize,
).(*connection)
streamManager = NewMockStreamManager(mockCtrl)
conn.streamsMap = streamManager
Expand Down Expand Up @@ -2589,6 +2590,7 @@ var _ = Describe("Client Connection", func() {
tr,
utils.DefaultLogger,
protocol.Version1,
protocol.DefaultMaxUDPPayloadSize,
).(*connection)
packer = NewMockPacker(mockCtrl)
conn.packer = packer
Expand Down
4 changes: 0 additions & 4 deletions interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,6 @@ type Config struct {
// 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
// MaxUDPPayloadSize configures the max_udp_payload_size transport parameter. This is the the limit on much data this
// endpoint is willing to receive.
// This is an experimental config option, it might be removed once PMTU can account for the path changing to lower values
MaxUDPPayloadSize 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.
Expand Down
2 changes: 2 additions & 0 deletions logging/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ const (
PacketDropUnexpectedVersion
// PacketDropDuplicate is used when a duplicate packet is received
PacketDropDuplicate
// PacketTooBig is used when packet size > max UDP payload
PacketTooBig
)

// TimerType is the type of the loss detection timer
Expand Down
9 changes: 7 additions & 2 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ type baseServer struct {
disableVersionNegotiation bool
acceptEarlyConns bool

tlsConf *tls.Config
config *Config
tlsConf *tls.Config
config *Config
maxUDPPayloadSize protocol.ByteCount

conn rawConn

Expand Down Expand Up @@ -98,6 +99,7 @@ type baseServer struct {
*logging.ConnectionTracer,
utils.Logger,
protocol.Version,
protocol.ByteCount, /* max UDP payload size */
) quicConn

closeMx sync.Mutex
Expand Down Expand Up @@ -244,12 +246,14 @@ func newServer(
verifySourceAddress func(net.Addr) bool,
disableVersionNegotiation bool,
acceptEarly bool,
maxUDPPayloadSize protocol.ByteCount,
) *baseServer {
s := &baseServer{
conn: conn,
connContext: connContext,
tlsConf: tlsConf,
config: config,
maxUDPPayloadSize: maxUDPPayloadSize,
tokenGenerator: handshake.NewTokenGenerator(tokenGeneratorKey),
maxTokenAge: maxTokenAge,
verifySourceAddress: verifySourceAddress,
Expand Down Expand Up @@ -689,6 +693,7 @@ func (s *baseServer) handleInitialImpl(p receivedPacket, hdr *wire.Header) error
tracer,
s.logger,
hdr.Version,
s.maxUDPPayloadSize,
)
conn.handlePacket(p)
// Adding the connection will fail if the client's chosen Destination Connection ID is already in use.
Expand Down
Loading

0 comments on commit 3e1fff0

Please sign in to comment.