Skip to content

Commit

Permalink
sendQueue: ignore "datagram too large" error
Browse files Browse the repository at this point in the history
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 quic-go#3273 quic-go#3327
  • Loading branch information
zllovesuki committed Feb 9, 2022
1 parent f3b0987 commit b84f687
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 32 deletions.
2 changes: 1 addition & 1 deletion conn_windows.go
Expand Up @@ -13,7 +13,7 @@ import (
)

const (
disablePathMTUDiscovery = true
disablePathMTUDiscovery = false
IP_DONTFRAGMENT = 14
)

Expand Down
18 changes: 18 additions & 0 deletions 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
}
11 changes: 11 additions & 0 deletions 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
}
16 changes: 16 additions & 0 deletions 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)
}
16 changes: 16 additions & 0 deletions 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/troubleshoot/windows-server/networking/wsaemsgsize-error-10040-in-winsock-2
return errors.Is(e, windows.WSAEMSGSIZE)
}
4 changes: 3 additions & 1 deletion send_queue.go
Expand Up @@ -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 {
Expand Down
57 changes: 27 additions & 30 deletions session_test.go
Expand Up @@ -9,7 +9,6 @@ import (
"fmt"
"io"
"net"
"runtime"
"runtime/pprof"
"strings"
"time"
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit b84f687

Please sign in to comment.