From ba9563a47a019d539e3fb79c824fe909b1c80466 Mon Sep 17 00:00:00 2001 From: Rachel Chen Date: Wed, 9 Feb 2022 04:04:24 -0800 Subject: [PATCH] sendQueue: ignore "datagram too large" error This commit introduces additional platform-dependent checking when the kernel returns an error. Previously, the session is terminated when PingFrame sends a discovery packet larger than the limit. With this commit, the error is checked, and if it is "datagram too large", the error is ignored. Additionally, this commit re-enables MTU discovery on Windows unless it is disabled explicitly by user. Fixes #3273 #3327 --- conn_windows.go | 2 +- err_size.go | 18 ++++++++++++++ err_size_generic.go | 11 +++++++++ err_size_unix.go | 16 +++++++++++++ err_size_windows.go | 16 +++++++++++++ send_queue.go | 4 +++- session_test.go | 57 +++++++++++++++++++++------------------------ 7 files changed, 92 insertions(+), 32 deletions(-) create mode 100644 err_size.go create mode 100644 err_size_generic.go create mode 100644 err_size_unix.go create mode 100644 err_size_windows.go diff --git a/conn_windows.go b/conn_windows.go index a6e591b62aa..f95e1439a9c 100644 --- a/conn_windows.go +++ b/conn_windows.go @@ -13,7 +13,7 @@ import ( ) const ( - disablePathMTUDiscovery = true + disablePathMTUDiscovery = false IP_DONTFRAGMENT = 14 ) diff --git a/err_size.go b/err_size.go new file mode 100644 index 00000000000..9e9552c4205 --- /dev/null +++ b/err_size.go @@ -0,0 +1,18 @@ +package quic + +import ( + "net" + "os" + "syscall" +) + +func isMsgSizeErr(err error) bool { + if opErr, ok := err.(*net.OpError); ok { + if syscallErr, ok := opErr.Err.(*os.SyscallError); ok { + if syscallErrno, ok := syscallErr.Err.(syscall.Errno); ok { + return assertErrorMsgSize(syscallErrno) + } + } + } + return false +} diff --git a/err_size_generic.go b/err_size_generic.go new file mode 100644 index 00000000000..13f1f64bfa8 --- /dev/null +++ b/err_size_generic.go @@ -0,0 +1,11 @@ +//go:build !unix && !windows +// +build !unix,!windows + +package quic + +import "syscall" + +func assertErrorMsgSize(e syscall.Errno) bool { + // to be implemented for more specific platforms + return false +} diff --git a/err_size_unix.go b/err_size_unix.go new file mode 100644 index 00000000000..47da3bc03ba --- /dev/null +++ b/err_size_unix.go @@ -0,0 +1,16 @@ +//go:build unix +// +build unix + +package quic + +import ( + "errors" + "syscall" + + "golang.org/x/sys/unix" +) + +func assertErrorMsgSize(e syscall.Errno) bool { + // https://man7.org/linux/man-pages/man7/udp.7.html + return errors.Is(e, unix.EMSGSIZE) +} diff --git a/err_size_windows.go b/err_size_windows.go new file mode 100644 index 00000000000..d9ca59c076b --- /dev/null +++ b/err_size_windows.go @@ -0,0 +1,16 @@ +//go:build windows +// +build windows + +package quic + +import ( + "errors" + "syscall" + + "golang.org/x/sys/windows" +) + +func assertErrorMsgSize(e syscall.Errno) bool { + // https://docs.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2 + return errors.Is(e, windows.WSAEMSGSIZE) +} diff --git a/send_queue.go b/send_queue.go index bf25dded60e..2f110cb9a29 100644 --- a/send_queue.go +++ b/send_queue.go @@ -64,7 +64,9 @@ func (h *sendQueue) Run() error { shouldClose = true case p := <-h.queue: if err := h.conn.Write(p.Data); err != nil { - return err + if !isMsgSizeErr(err) { + return err + } } p.Release() select { diff --git a/session_test.go b/session_test.go index 7ca60947f55..531828a0920 100644 --- a/session_test.go +++ b/session_test.go @@ -9,7 +9,6 @@ import ( "fmt" "io" "net" - "runtime" "runtime/pprof" "strings" "time" @@ -1682,35 +1681,33 @@ var _ = Describe("Session", func() { time.Sleep(50 * time.Millisecond) }) - if runtime.GOOS != "windows" { // Path MTU Discovery is disabled on Windows - It("sends a Path MTU probe packet", func() { - mtuDiscoverer := NewMockMtuDiscoverer(mockCtrl) - sess.mtuDiscoverer = mtuDiscoverer - sess.config.DisablePathMTUDiscovery = false - sph.EXPECT().SentPacket(gomock.Any()) - sph.EXPECT().HasPacingBudget().Return(true).AnyTimes() - sph.EXPECT().SendMode().Return(ackhandler.SendAny) - sph.EXPECT().SendMode().Return(ackhandler.SendNone) - written := make(chan struct{}, 1) - sender.EXPECT().WouldBlock().AnyTimes() - sender.EXPECT().Send(gomock.Any()).DoAndReturn(func(p *packetBuffer) { written <- struct{}{} }) - gomock.InOrder( - mtuDiscoverer.EXPECT().NextProbeTime(), - mtuDiscoverer.EXPECT().ShouldSendProbe(gomock.Any()).Return(true), - mtuDiscoverer.EXPECT().NextProbeTime(), - ) - ping := ackhandler.Frame{Frame: &wire.PingFrame{}} - mtuDiscoverer.EXPECT().GetPing().Return(ping, protocol.ByteCount(1234)) - packer.EXPECT().PackMTUProbePacket(ping, protocol.ByteCount(1234)).Return(getPacket(1), nil) - go func() { - defer GinkgoRecover() - cryptoSetup.EXPECT().RunHandshake().MaxTimes(1) - sess.run() - }() - sess.scheduleSending() - Eventually(written).Should(Receive()) - }) - } + It("sends a Path MTU probe packet", func() { + mtuDiscoverer := NewMockMtuDiscoverer(mockCtrl) + sess.mtuDiscoverer = mtuDiscoverer + sess.config.DisablePathMTUDiscovery = false + sph.EXPECT().SentPacket(gomock.Any()) + sph.EXPECT().HasPacingBudget().Return(true).AnyTimes() + sph.EXPECT().SendMode().Return(ackhandler.SendAny) + sph.EXPECT().SendMode().Return(ackhandler.SendNone) + written := make(chan struct{}, 1) + sender.EXPECT().WouldBlock().AnyTimes() + sender.EXPECT().Send(gomock.Any()).DoAndReturn(func(p *packetBuffer) { written <- struct{}{} }) + gomock.InOrder( + mtuDiscoverer.EXPECT().NextProbeTime(), + mtuDiscoverer.EXPECT().ShouldSendProbe(gomock.Any()).Return(true), + mtuDiscoverer.EXPECT().NextProbeTime(), + ) + ping := ackhandler.Frame{Frame: &wire.PingFrame{}} + mtuDiscoverer.EXPECT().GetPing().Return(ping, protocol.ByteCount(1234)) + packer.EXPECT().PackMTUProbePacket(ping, protocol.ByteCount(1234)).Return(getPacket(1), nil) + go func() { + defer GinkgoRecover() + cryptoSetup.EXPECT().RunHandshake().MaxTimes(1) + sess.run() + }() + sess.scheduleSending() + Eventually(written).Should(Receive()) + }) }) Context("scheduling sending", func() {