From 0eb3f14a60ed5168895612584a11b55786db01f8 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 2 Dec 2020 15:42:29 +0700 Subject: [PATCH 1/2] only print the warning about the UDP receive buffer size once --- packet_handler_map.go | 31 +++++++++++++++++-------------- packet_handler_map_test.go | 6 ++++++ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/packet_handler_map.go b/packet_handler_map.go index ae542e59..e158b53e 100644 --- a/packet_handler_map.go +++ b/packet_handler_map.go @@ -4,6 +4,7 @@ import ( "crypto/hmac" "crypto/rand" "crypto/sha256" + "errors" "fmt" "hash" "log" @@ -54,40 +55,38 @@ type packetHandlerMap struct { var _ packetHandlerManager = &packetHandlerMap{} -func setReceiveBuffer(c net.PacketConn, logger utils.Logger) { +func setReceiveBuffer(c net.PacketConn, logger utils.Logger) error { conn, ok := c.(interface{ SetReadBuffer(int) error }) if !ok { - logger.Debugf("Connection doesn't allow setting of receive buffer size") - return + return errors.New("connection doesn't allow setting of receive buffer size") } size, err := inspectReadBuffer(c) if err != nil { - log.Printf("Failed to determine receive buffer size: %s", err) - return + return fmt.Errorf("failed to determine receive buffer size: %w", err) } if size >= protocol.DesiredReceiveBufferSize { logger.Debugf("Conn has receive buffer of %d kiB (wanted: at least %d kiB)", size/1024, protocol.DesiredReceiveBufferSize/1024) } if err := conn.SetReadBuffer(protocol.DesiredReceiveBufferSize); err != nil { - log.Printf("Failed to increase receive buffer size: %s\n", err) - return + return fmt.Errorf("failed to increase receive buffer size: %w", err) } newSize, err := inspectReadBuffer(c) if err != nil { - log.Printf("Failed to determine receive buffer size: %s", err) - return + return fmt.Errorf("failed to determine receive buffer size: %w", err) } if newSize == size { - log.Printf("Failed to determine receive buffer size: %s", err) - return + return fmt.Errorf("failed to determine receive buffer size: %w", err) } if newSize < protocol.DesiredReceiveBufferSize { - log.Printf("Failed to sufficiently increase receive buffer size. Was: %d kiB, wanted: %d kiB, got: %d kiB.", size/1024, protocol.DesiredReceiveBufferSize/1024, newSize/1024) - return + return fmt.Errorf("failed to sufficiently increase receive buffer size (was: %d kiB, wanted: %d kiB, got: %d kiB)", size/1024, protocol.DesiredReceiveBufferSize/1024, newSize/1024) } logger.Debugf("Increased receive buffer size to %d kiB", newSize/1024) + return nil } +// only print warnings about the UPD receive buffer size once +var receiveBufferWarningOnce sync.Once + func newPacketHandlerMap( c net.PacketConn, connIDLen int, @@ -95,7 +94,11 @@ func newPacketHandlerMap( tracer logging.Tracer, logger utils.Logger, ) (packetHandlerManager, error) { - setReceiveBuffer(c, logger) + if err := setReceiveBuffer(c, logger); err != nil { + receiveBufferWarningOnce.Do(func() { + log.Println(err) + }) + } conn, err := wrapConn(c) if err != nil { return nil, err diff --git a/packet_handler_map_test.go b/packet_handler_map_test.go index 06cb9b8f..86dd7924 100644 --- a/packet_handler_map_test.go +++ b/packet_handler_map_test.go @@ -4,7 +4,10 @@ import ( "bytes" "crypto/rand" "errors" + "io/ioutil" + "log" "net" + "os" "time" mocklogging "github.com/lucas-clemente/quic-go/internal/mocks/logging" @@ -63,6 +66,7 @@ var _ = Describe("Packet Handler Map", func() { }) JustBeforeEach(func() { + log.SetOutput(ioutil.Discard) conn = NewMockPacketConn(mockCtrl) conn.EXPECT().LocalAddr().Return(&net.UDPAddr{}).AnyTimes() conn.EXPECT().ReadFrom(gomock.Any()).DoAndReturn(func(b []byte) (int, net.Addr, error) { @@ -77,6 +81,8 @@ var _ = Describe("Packet Handler Map", func() { handler = phm.(*packetHandlerMap) }) + AfterEach(func() { log.SetOutput(os.Stdout) }) + It("closes", func() { getMultiplexer() // make the sync.Once execute // replace the clientMuxer. getClientMultiplexer will now return the MockMultiplexer From b0974c14ad18d76286bc68c78e7495e677b495dc Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 2 Dec 2020 15:52:24 +0700 Subject: [PATCH 2/2] link to the wiki explaining the UDP receive buffer size --- packet_handler_map.go | 2 +- packet_handler_map_test.go | 6 ------ quic_suite_test.go | 6 ++++++ 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packet_handler_map.go b/packet_handler_map.go index e158b53e..3312e377 100644 --- a/packet_handler_map.go +++ b/packet_handler_map.go @@ -96,7 +96,7 @@ func newPacketHandlerMap( ) (packetHandlerManager, error) { if err := setReceiveBuffer(c, logger); err != nil { receiveBufferWarningOnce.Do(func() { - log.Println(err) + log.Printf("%s. See https://github.com/lucas-clemente/quic-go/wiki/UDP-Receive-Buffer-Size for details.", err) }) } conn, err := wrapConn(c) diff --git a/packet_handler_map_test.go b/packet_handler_map_test.go index 86dd7924..06cb9b8f 100644 --- a/packet_handler_map_test.go +++ b/packet_handler_map_test.go @@ -4,10 +4,7 @@ import ( "bytes" "crypto/rand" "errors" - "io/ioutil" - "log" "net" - "os" "time" mocklogging "github.com/lucas-clemente/quic-go/internal/mocks/logging" @@ -66,7 +63,6 @@ var _ = Describe("Packet Handler Map", func() { }) JustBeforeEach(func() { - log.SetOutput(ioutil.Discard) conn = NewMockPacketConn(mockCtrl) conn.EXPECT().LocalAddr().Return(&net.UDPAddr{}).AnyTimes() conn.EXPECT().ReadFrom(gomock.Any()).DoAndReturn(func(b []byte) (int, net.Addr, error) { @@ -81,8 +77,6 @@ var _ = Describe("Packet Handler Map", func() { handler = phm.(*packetHandlerMap) }) - AfterEach(func() { log.SetOutput(os.Stdout) }) - It("closes", func() { getMultiplexer() // make the sync.Once execute // replace the clientMuxer. getClientMultiplexer will now return the MockMultiplexer diff --git a/quic_suite_test.go b/quic_suite_test.go index a26b3504..d8a0a2e0 100644 --- a/quic_suite_test.go +++ b/quic_suite_test.go @@ -1,6 +1,8 @@ package quic import ( + "io/ioutil" + "log" "sync" "testing" @@ -23,6 +25,10 @@ var _ = BeforeEach(func() { connMuxerOnce = *new(sync.Once) }) +var _ = BeforeSuite(func() { + log.SetOutput(ioutil.Discard) +}) + var _ = AfterEach(func() { mockCtrl.Finish() })