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,
1. This commit re-enables MTU discovery on Windows unless it
is disabled explicitly by user (Undo quic-go#3276),
2. Set IP_DONTFRAGMENT and IPV6_DONTFRAG with error checking on Windows,
   and
3. Set IP_MTU_DISCOVERY to PMTUDISC_DO for both IPv4 and IPv6 on Linux
   so that the kernel will return "message too long".

Fixes quic-go#3273 quic-go#3327

conn_generic.go

@@ -5,8 +5,6 @@ package quic
 
 import "net"
 
-const disablePathMTUDiscovery = false
-
 func newConn(c net.PacketConn) (connection, error) {
 	return &basicConn{PacketConn: c}, nil
 }

conn_helper_darwin.go

@@ -5,10 +5,7 @@ package quic
 
 import "golang.org/x/sys/unix"
 
-const (
-	msgTypeIPTOS            = unix.IP_RECVTOS
-	disablePathMTUDiscovery = false
-)
+const msgTypeIPTOS = unix.IP_RECVTOS
 
 const (
 	ipv4RECVPKTINFO = unix.IP_RECVPKTINFO

conn_helper_freebsd.go

@@ -6,8 +6,7 @@ package quic
 import "golang.org/x/sys/unix"
 
 const (
-	msgTypeIPTOS            = unix.IP_RECVTOS
-	disablePathMTUDiscovery = false
+	msgTypeIPTOS = unix.IP_RECVTOS
 )
 
 const (

conn_helper_linux.go

@@ -5,10 +5,7 @@ package quic
 
 import "golang.org/x/sys/unix"
 
-const (
-	msgTypeIPTOS            = unix.IP_TOS
-	disablePathMTUDiscovery = false
-)
+const msgTypeIPTOS = unix.IP_TOS
 
 const (
 	ipv4RECVPKTINFO = unix.IP_PKTINFO

conn_oob.go

@@ -87,6 +87,8 @@ func newConn(c OOBCapablePacketConn) (*oobConn, error) {
 			errPIIPv4 = unix.SetsockoptInt(int(fd), unix.IPPROTO_IP, ipv4RECVPKTINFO, 1)
 			errPIIPv6 = unix.SetsockoptInt(int(fd), unix.IPPROTO_IPV6, ipv6RECVPKTINFO, 1)
 		}
+
+		setOOBSockOpts(fd)
 	}); err != nil {
 		return nil, err
 	}

conn_oob_opts.go

@@ -0,0 +1,8 @@
+//go:build !linux && !windows
+// +build !linux,!windows
+
+package quic
+
+func setOOBSockOpts(fd uintptr) {
+	// no-op on unsupported platforms
+}

conn_oob_opts_linux.go

@@ -0,0 +1,15 @@
+//go:build linux
+// +build linux
+
+package quic
+
+import (
+	"golang.org/x/sys/unix"
+)
+
+func setOOBSockOpts(fd uintptr) {
+	// Enabling IP_MTU_DISCOVER will force the kernel to return "sendto: message too long"
+	// and the datagram will not be fragmented
+	unix.SetsockoptInt(int(fd), unix.IPPROTO_IP, unix.IP_MTU_DISCOVER, unix.IP_PMTUDISC_DO)
+	unix.SetsockoptInt(int(fd), unix.IPPROTO_IPV6, unix.IPV6_MTU_DISCOVER, unix.IPV6_PMTUDISC_DO)
+}

conn_windows.go

@@ -9,27 +9,40 @@ import (
 	"net"
 	"syscall"
 
+	"github.com/lucas-clemente/quic-go/internal/utils"
 	"golang.org/x/sys/windows"
 )
 
 const (
-	disablePathMTUDiscovery = true
-	IP_DONTFRAGMENT         = 14
+	// same for both IPv4 and IPv6 on Windows
+	// https://microsoft.github.io/windows-docs-rs/doc/windows/Win32/Networking/WinSock/constant.IP_DONTFRAG.html
+	// https://microsoft.github.io/windows-docs-rs/doc/windows/Win32/Networking/WinSock/constant.IPV6_DONTFRAG.html
+	IP_DONTFRAGMENT = 14
+	IPV6_DONTFRAG   = 14
 )
 
 func newConn(c OOBCapablePacketConn) (connection, error) {
 	rawConn, err := c.SyscallConn()
 	if err != nil {
 		return nil, fmt.Errorf("couldn't get syscall.RawConn: %w", err)
 	}
+	var errDFIPv4, errDFIPv6 error
 	if err := rawConn.Control(func(fd uintptr) {
-		// This should succeed if the connection is a IPv4 or a dual-stack connection.
-		// It will fail for IPv6 connections.
-		// TODO: properly handle error.
-		_ = windows.SetsockoptInt(windows.Handle(fd), windows.IPPROTO_IP, IP_DONTFRAGMENT, 1)
+		errDFIPv4 = windows.SetsockoptInt(windows.Handle(fd), windows.IPPROTO_IP, IP_DONTFRAGMENT, 1)
+		errDFIPv6 = windows.SetsockoptInt(windows.Handle(fd), windows.IPPROTO_IPV6, IPV6_DONTFRAG, 1)
 	}); err != nil {
 		return nil, err
 	}
+	switch {
+	case errDFIPv4 == nil && errDFIPv6 == nil:
+		utils.DefaultLogger.Debugf("Setting DF for IPv4 and IPv6.")
+	case errDFIPv4 == nil && errDFIPv6 != nil:
+		utils.DefaultLogger.Debugf("Setting DF for IPv4.")
+	case errDFIPv4 != nil && errDFIPv6 == nil:
+		utils.DefaultLogger.Debugf("Setting DF for IPv6.")
+	case errDFIPv4 != nil && errDFIPv6 != nil:
+		return nil, errors.New("setting Df failed for both IPv4 and IPv6")
+	}
 	return &basicConn{PacketConn: c}, nil
 }
 

err_size.go

@@ -0,0 +1,9 @@
+//go:build !linux && !windows
+// +build !linux,!windows
+
+package quic
+
+func isMsgSizeErr(err error) bool {
+	// to be implemented for more specific platforms
+	return false
+}

err_size_linux.go

@@ -0,0 +1,15 @@
+//go:build linux
+// +build linux
+
+package quic
+
+import (
+	"errors"
+
+	"golang.org/x/sys/unix"
+)
+
+func isMsgSizeErr(err error) bool {
+	// https://man7.org/linux/man-pages/man7/udp.7.html
+	return errors.Is(err, unix.EMSGSIZE)
+}

err_size_windows.go

@@ -0,0 +1,15 @@
+//go:build windows
+// +build windows
+
+package quic
+
+import (
+	"errors"
+
+	"golang.org/x/sys/windows"
+)
+
+func isMsgSizeErr(err error) bool {
+	// https://docs.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2
+	return errors.Is(err, windows.WSAEMSGSIZE)
+}

interface.go

@@ -290,7 +290,7 @@ type Config struct {
 	KeepAlive bool
 	// DisablePathMTUDiscovery disables Path MTU Discovery (RFC 8899).
 	// Packets will then be at most 1252 (IPv4) / 1232 (IPv6) bytes in size.
-	// Note that Path MTU discovery is always disabled on Windows, see https://github.com/lucas-clemente/quic-go/issues/3273.
+	// Note that if Path MTU discovery is causing issues on your system, please open a new issue
 	DisablePathMTUDiscovery bool
 	// DisableVersionNegotiationPackets disables the sending of Version Negotiation packets.
 	// This can be useful if version information is exchanged out-of-band.

send_queue.go

@@ -64,7 +64,13 @@ func (h *sendQueue) Run() error {
 			shouldClose = true
 		case p := <-h.queue:
 			if err := h.conn.Write(p.Data); err != nil {
-				return err
+				// This additional check enables:
+				// 1. Checking for "datagram too large" message from the kernel, as such,
+				// 2. Path MTU discovery,and
+				// 3. Eventual detection of loss PingFrame.
+				if !isMsgSizeErr(err) {
+					return err
+				}
 			}
 			p.Release()
 			select {

session.go

@@ -130,10 +130,6 @@ func (e *errCloseForRecreating) Error() string {
 var sessionTracingID uint64        // to be accessed atomically
 func nextSessionTracingID() uint64 { return atomic.AddUint64(&sessionTracingID, 1) }
 
-func pathMTUDiscoveryEnabled(config *Config) bool {
-	return !disablePathMTUDiscovery && !config.DisablePathMTUDiscovery
-}
-
 // A Session is a QUIC session
 type session struct {
 	// Destination connection ID used during the handshake.
@@ -755,7 +751,7 @@ func (s *session) maybeResetTimer() {
 			deadline = s.idleTimeoutStartTime().Add(s.idleTimeout)
 		}
 	}
-	if s.handshakeConfirmed && pathMTUDiscoveryEnabled(s.config) {
+	if s.handshakeConfirmed && !s.config.DisablePathMTUDiscovery {
 		if probeTime := s.mtuDiscoverer.NextProbeTime(); !probeTime.IsZero() {
 			deadline = utils.MinTime(deadline, probeTime)
 		}
@@ -819,7 +815,7 @@ func (s *session) handleHandshakeConfirmed() {
 	s.sentPacketHandler.SetHandshakeConfirmed()
 	s.cryptoStreamHandler.SetHandshakeConfirmed()
 
-	if pathMTUDiscoveryEnabled(s.config) {
+	if !s.config.DisablePathMTUDiscovery {
 		maxPacketSize := s.peerParams.MaxUDPPayloadSize
 		if maxPacketSize == 0 {
 			maxPacketSize = protocol.MaxByteCount
@@ -1780,7 +1776,7 @@ func (s *session) sendPacket() (bool, error) {
 		s.sendQueue.Send(packet.buffer)
 		return true, nil
 	}
-	if pathMTUDiscoveryEnabled(s.config) && s.mtuDiscoverer.ShouldSendProbe(now) {
+	if !s.config.DisablePathMTUDiscovery && s.mtuDiscoverer.ShouldSendProbe(now) {
 		packet, err := s.packer.PackMTUProbePacket(s.mtuDiscoverer.GetPing())
 		if err != nil {
 			return false, err

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() {