Merge pull request #3107 from lucas-clemente/fix-cubic-initialization

initialize the congestion controller with the actual max datagram size

internal/ackhandler/ackhandler.go

@@ -9,12 +9,13 @@ import (
 // NewAckHandler creates a new SentPacketHandler and a new ReceivedPacketHandler
 func NewAckHandler(
 	initialPacketNumber protocol.PacketNumber,
+	initialMaxDatagramSize protocol.ByteCount,
 	rttStats *utils.RTTStats,
 	pers protocol.Perspective,
 	tracer logging.ConnectionTracer,
 	logger utils.Logger,
 	version protocol.VersionNumber,
 ) (SentPacketHandler, ReceivedPacketHandler) {
-	sph := newSentPacketHandler(initialPacketNumber, rttStats, pers, tracer, logger)
+	sph := newSentPacketHandler(initialPacketNumber, initialMaxDatagramSize, rttStats, pers, tracer, logger)
 	return sph, newReceivedPacketHandler(sph, rttStats, logger, version)
 }

internal/ackhandler/sent_packet_handler.go

@@ -101,6 +101,7 @@ var (
 
 func newSentPacketHandler(
 	initialPN protocol.PacketNumber,
+	initialMaxDatagramSize protocol.ByteCount,
 	rttStats *utils.RTTStats,
 	pers protocol.Perspective,
 	tracer logging.ConnectionTracer,
@@ -109,6 +110,7 @@ func newSentPacketHandler(
 	congestion := congestion.NewCubicSender(
 		congestion.DefaultClock{},
 		rttStats,
+		initialMaxDatagramSize,
 		true, // use Reno
 		tracer,
 	)

internal/ackhandler/sent_packet_handler_test.go

@@ -27,7 +27,7 @@ var _ = Describe("SentPacketHandler", func() {
 	JustBeforeEach(func() {
 		lostPackets = nil
 		rttStats := utils.NewRTTStats()
-		handler = newSentPacketHandler(42, rttStats, perspective, nil, utils.DefaultLogger)
+		handler = newSentPacketHandler(42, protocol.InitialPacketSizeIPv4, rttStats, perspective, nil, utils.DefaultLogger)
 		streamFrame = wire.StreamFrame{
 			StreamID: 5,
 			Data:     []byte{0x13, 0x37},

internal/congestion/cubic_sender.go

@@ -1,6 +1,7 @@
 package congestion
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/lucas-clemente/quic-go/internal/protocol"
@@ -14,9 +15,8 @@ const (
 	initialMaxDatagramSize     = protocol.ByteCount(protocol.InitialPacketSizeIPv4)
 	maxBurstPackets            = 3
 	renoBeta                   = 0.7 // Reno backoff factor.
-	initialMaxCongestionWindow = protocol.MaxCongestionWindowPackets * initialMaxDatagramSize
 	minCongestionWindowPackets = 2
-	initialCongestionWindow    = 32 * initialMaxDatagramSize
+	initialCongestionWindow    = 32
 )
 
 type cubicSender struct {
@@ -65,11 +65,33 @@ var (
 )
 
 // NewCubicSender makes a new cubic sender
-func NewCubicSender(clock Clock, rttStats *utils.RTTStats, reno bool, tracer logging.ConnectionTracer) *cubicSender {
+func NewCubicSender(
-	return newCubicSender(clock, rttStats, reno, initialCongestionWindow, initialMaxCongestionWindow, tracer)
+	clock Clock,
+	rttStats *utils.RTTStats,
+	initialMaxDatagramSize protocol.ByteCount,
+	reno bool,
+	tracer logging.ConnectionTracer,
+) *cubicSender {
+	return newCubicSender(
+		clock,
+		rttStats,
+		reno,
+		initialMaxDatagramSize,
+		initialCongestionWindow*initialMaxDatagramSize,
+		protocol.MaxCongestionWindowPackets*initialMaxDatagramSize,
+		tracer,
+	)
 }
 
-func newCubicSender(clock Clock, rttStats *utils.RTTStats, reno bool, initialCongestionWindow, initialMaxCongestionWindow protocol.ByteCount, tracer logging.ConnectionTracer) *cubicSender {
+func newCubicSender(
+	clock Clock,
+	rttStats *utils.RTTStats,
+	reno bool,
+	initialMaxDatagramSize,
+	initialCongestionWindow,
+	initialMaxCongestionWindow protocol.ByteCount,
+	tracer logging.ConnectionTracer,
+) *cubicSender {
 	c := &cubicSender{
 		rttStats:                   rttStats,
 		largestSentPacketNumber:    protocol.InvalidPacketNumber,
@@ -283,7 +305,7 @@ func (c *cubicSender) maybeTraceStateChange(new logging.CongestionState) {
 
 func (c *cubicSender) SetMaxDatagramSize(s protocol.ByteCount) {
 	if s < c.maxDatagramSize {
-		panic("congestion BUG: decreased max datagram size")
+		panic(fmt.Sprintf("congestion BUG: decreased max datagram size from %d to %d", c.maxDatagramSize, s))
 	}
 	cwndIsMinCwnd := c.congestionWindow == c.minCongestionWindow()
 	c.maxDatagramSize = s

internal/congestion/cubic_sender_test.go

@@ -42,7 +42,15 @@ var _ = Describe("Cubic Sender", func() {
 		ackedPacketNumber = 0
 		clock = mockClock{}
 		rttStats = utils.NewRTTStats()
-		sender = newCubicSender(&clock, rttStats, true /*reno*/, initialCongestionWindowPackets*maxDatagramSize, MaxCongestionWindow, nil)
+		sender = newCubicSender(
+			&clock,
+			rttStats,
+			true, /*reno*/
+			protocol.InitialPacketSizeIPv4,
+			initialCongestionWindowPackets*maxDatagramSize,
+			MaxCongestionWindow,
+			nil,
+		)
 	})
 
 	SendAvailableSendWindowLen := func(packetLength protocol.ByteCount) int {
@@ -310,7 +318,7 @@ var _ = Describe("Cubic Sender", func() {
 	It("tcp cubic reset epoch on quiescence", func() {
 		const maxCongestionWindow = 50
 		const maxCongestionWindowBytes = maxCongestionWindow * maxDatagramSize
-		sender = newCubicSender(&clock, rttStats, false, initialCongestionWindowPackets*maxDatagramSize, maxCongestionWindowBytes, nil)
+		sender = newCubicSender(&clock, rttStats, false, protocol.InitialPacketSizeIPv4, initialCongestionWindowPackets*maxDatagramSize, maxCongestionWindowBytes, nil)
 
 		numSent := SendAvailableSendWindow()
 
@@ -450,7 +458,8 @@ var _ = Describe("Cubic Sender", func() {
 	})
 
 	It("slow starts up to the maximum congestion window", func() {
-		sender = newCubicSender(&clock, rttStats, true, initialCongestionWindowPackets*maxDatagramSize, initialMaxCongestionWindow, nil)
+		const initialMaxCongestionWindow = protocol.MaxCongestionWindowPackets * initialMaxDatagramSize
+		sender = newCubicSender(&clock, rttStats, true, protocol.InitialPacketSizeIPv4, initialCongestionWindowPackets*maxDatagramSize, initialMaxCongestionWindow, nil)
 
 		for i := 1; i < protocol.MaxCongestionWindowPackets; i++ {
 			sender.MaybeExitSlowStart()
@@ -464,7 +473,8 @@ var _ = Describe("Cubic Sender", func() {
 	})
 
 	It("slow starts up to maximum congestion window, if larger packets are sent", func() {
-		sender = newCubicSender(&clock, rttStats, true, initialCongestionWindowPackets*maxDatagramSize, initialMaxCongestionWindow, nil)
+		const initialMaxCongestionWindow = protocol.MaxCongestionWindowPackets * initialMaxDatagramSize
+		sender = newCubicSender(&clock, rttStats, true, protocol.InitialPacketSizeIPv4, initialCongestionWindowPackets*maxDatagramSize, initialMaxCongestionWindow, nil)
 		const packetSize = initialMaxDatagramSize + 100
 		sender.SetMaxDatagramSize(packetSize)
 		for i := 1; i < protocol.MaxCongestionWindowPackets; i++ {
@@ -479,7 +489,7 @@ var _ = Describe("Cubic Sender", func() {
 
 	It("limit cwnd increase in congestion avoidance", func() {
 		// Enable Cubic.
-		sender = newCubicSender(&clock, rttStats, false, initialCongestionWindowPackets*maxDatagramSize, MaxCongestionWindow, nil)
+		sender = newCubicSender(&clock, rttStats, false, protocol.InitialPacketSizeIPv4, initialCongestionWindowPackets*maxDatagramSize, MaxCongestionWindow, nil)
 		numSent := SendAvailableSendWindow()
 
 		// Make sure we fall out of slow start.

session.go

@@ -293,6 +293,7 @@ var newSession = func(
 	s.preSetup()
 	s.sentPacketHandler, s.receivedPacketHandler = ackhandler.NewAckHandler(
 		0,
+		getMaxPacketSize(s.conn.RemoteAddr()),
 		s.rttStats,
 		s.perspective,
 		s.tracer,
@@ -417,6 +418,7 @@ var newClientSession = func(
 	s.preSetup()
 	s.sentPacketHandler, s.receivedPacketHandler = ackhandler.NewAckHandler(
 		initialPacketNumber,
+		getMaxPacketSize(s.conn.RemoteAddr()),
 		s.rttStats,
 		s.perspective,
 		s.tracer,