From 614fdb3271b4e197db34570b9a43d4e19b34e12a Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 2 Jun 2023 16:54:34 +0300 Subject: [PATCH] only run DPLPMTUD if the connection can send packets with the DF bit set (#3879) --- connection.go | 4 ++-- connection_test.go | 1 + interface.go | 5 +++-- mock_send_conn_test.go | 14 ++++++++++++++ packet_handler_map.go | 8 ++++++++ send_conn.go | 2 ++ sys_conn.go | 11 ++++++++--- sys_conn_df.go | 4 ++-- sys_conn_df_linux.go | 8 ++++---- sys_conn_df_windows.go | 8 ++++---- sys_conn_no_oob.go | 4 ++-- sys_conn_oob.go | 9 ++++++++- sys_conn_oob_test.go | 5 +++-- sys_conn_windows.go | 4 ++-- sys_conn_windows_test.go | 6 ++++-- 15 files changed, 67 insertions(+), 26 deletions(-) diff --git a/connection.go b/connection.go index 67a5df54..ecb54ac0 100644 --- a/connection.go +++ b/connection.go @@ -800,7 +800,7 @@ func (s *connection) handleHandshakeConfirmed() { s.sentPacketHandler.SetHandshakeConfirmed() s.cryptoStreamHandler.SetHandshakeConfirmed() - if !s.config.DisablePathMTUDiscovery { + if !s.config.DisablePathMTUDiscovery && s.conn.capabilities().DF { maxPacketSize := s.peerParams.MaxUDPPayloadSize if maxPacketSize == 0 { maxPacketSize = protocol.MaxByteCount @@ -1887,7 +1887,7 @@ func (s *connection) sendPacket() (bool, error) { s.sentFirstPacket = true s.sendPackedCoalescedPacket(packet, now) return true, nil - } else if !s.config.DisablePathMTUDiscovery && s.mtuDiscoverer.ShouldSendProbe(now) { + } else if s.mtuDiscoverer != nil && s.mtuDiscoverer.ShouldSendProbe(now) { ping, size := s.mtuDiscoverer.GetPing() p, buffer, err := s.packer.PackMTUProbePacket(ping, size, now, s.version) if err != nil { diff --git a/connection_test.go b/connection_test.go index 3dc1f923..ea79c9a8 100644 --- a/connection_test.go +++ b/connection_test.go @@ -2387,6 +2387,7 @@ var _ = Describe("Client Connection", func() { mconn = NewMockSendConn(mockCtrl) mconn.EXPECT().RemoteAddr().Return(&net.UDPAddr{}).AnyTimes() mconn.EXPECT().LocalAddr().Return(&net.UDPAddr{}).AnyTimes() + mconn.EXPECT().capabilities().AnyTimes() if tlsConf == nil { tlsConf = &tls.Config{} } diff --git a/interface.go b/interface.go index d98807b1..1635619f 100644 --- a/interface.go +++ b/interface.go @@ -314,8 +314,9 @@ type Config struct { // every half of MaxIdleTimeout, whichever is smaller). KeepAlivePeriod time.Duration // DisablePathMTUDiscovery disables Path MTU Discovery (RFC 8899). - // Packets will then be at most 1252 (IPv4) / 1232 (IPv6) bytes in size. - // Note that if Path MTU discovery is causing issues on your system, please open a new issue + // This allows the sending of QUIC packets that fully utilize the available MTU of the path. + // Path MTU discovery is only available on systems that allow setting of the Don't Fragment (DF) bit. + // If unavailable or disabled, packets will be at most 1252 (IPv4) / 1232 (IPv6) bytes in size. DisablePathMTUDiscovery bool // DisableVersionNegotiationPackets disables the sending of Version Negotiation packets. // This can be useful if version information is exchanged out-of-band. diff --git a/mock_send_conn_test.go b/mock_send_conn_test.go index a0fb66f0..ac6c3d15 100644 --- a/mock_send_conn_test.go +++ b/mock_send_conn_test.go @@ -89,3 +89,17 @@ func (mr *MockSendConnMockRecorder) Write(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Write", reflect.TypeOf((*MockSendConn)(nil).Write), arg0) } + +// capabilities mocks base method. +func (m *MockSendConn) capabilities() connCapabilities { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "capabilities") + ret0, _ := ret[0].(connCapabilities) + return ret0 +} + +// capabilities indicates an expected call of capabilities. +func (mr *MockSendConnMockRecorder) capabilities() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "capabilities", reflect.TypeOf((*MockSendConn)(nil).capabilities)) +} diff --git a/packet_handler_map.go b/packet_handler_map.go index 83caa192..05f3eff2 100644 --- a/packet_handler_map.go +++ b/packet_handler_map.go @@ -15,6 +15,12 @@ import ( "github.com/quic-go/quic-go/internal/utils" ) +type connCapabilities struct { + // This connection has the Don't Fragment (DF) bit set. + // This means it makes to run DPLPMTUD. + DF bool +} + // rawConn is a connection that allow reading of a receivedPackeh. type rawConn interface { ReadPacket() (*receivedPacket, error) @@ -22,6 +28,8 @@ type rawConn interface { LocalAddr() net.Addr SetReadDeadline(time.Time) error io.Closer + + capabilities() connCapabilities } type closePacket struct { diff --git a/send_conn.go b/send_conn.go index 0ac27037..399e1154 100644 --- a/send_conn.go +++ b/send_conn.go @@ -10,6 +10,8 @@ type sendConn interface { Close() error LocalAddr() net.Addr RemoteAddr() net.Addr + + capabilities() connCapabilities } type sconn struct { diff --git a/sys_conn.go b/sys_conn.go index d6c1d616..59b730fa 100644 --- a/sys_conn.go +++ b/sys_conn.go @@ -25,6 +25,7 @@ func wrapConn(pc net.PacketConn) (rawConn, error) { conn, ok := pc.(interface { SyscallConn() (syscall.RawConn, error) }) + var supportsDF bool if ok { rawConn, err := conn.SyscallConn() if err != nil { @@ -33,7 +34,8 @@ func wrapConn(pc net.PacketConn) (rawConn, error) { if _, ok := pc.LocalAddr().(*net.UDPAddr); ok { // Only set DF on sockets that we expect to be able to handle that configuration. - err = setDF(rawConn) + var err error + supportsDF, err = setDF(rawConn) if err != nil { return nil, err } @@ -42,9 +44,9 @@ func wrapConn(pc net.PacketConn) (rawConn, error) { c, ok := pc.(OOBCapablePacketConn) if !ok { utils.DefaultLogger.Infof("PacketConn is not a net.UDPConn. Disabling optimizations possible on UDP connections.") - return &basicConn{PacketConn: pc}, nil + return &basicConn{PacketConn: pc, supportsDF: supportsDF}, nil } - return newConn(c) + return newConn(c, supportsDF) } // The basicConn is the most trivial implementation of a connection. @@ -54,6 +56,7 @@ func wrapConn(pc net.PacketConn) (rawConn, error) { // * when the OS doesn't support OOB. type basicConn struct { net.PacketConn + supportsDF bool } var _ rawConn = &basicConn{} @@ -78,3 +81,5 @@ func (c *basicConn) ReadPacket() (*receivedPacket, error) { func (c *basicConn) WritePacket(b []byte, addr net.Addr, _ []byte) (n int, err error) { return c.PacketConn.WriteTo(b, addr) } + +func (c *basicConn) capabilities() connCapabilities { return connCapabilities{DF: c.supportsDF} } diff --git a/sys_conn_df.go b/sys_conn_df.go index ef9f981a..7e94b80e 100644 --- a/sys_conn_df.go +++ b/sys_conn_df.go @@ -4,9 +4,9 @@ package quic import "syscall" -func setDF(rawConn syscall.RawConn) error { +func setDF(syscall.RawConn) (bool, error) { // no-op on unsupported platforms - return nil + return false, nil } func isMsgSizeErr(err error) bool { diff --git a/sys_conn_df_linux.go b/sys_conn_df_linux.go index 98542b41..fe75ab3d 100644 --- a/sys_conn_df_linux.go +++ b/sys_conn_df_linux.go @@ -11,7 +11,7 @@ import ( "github.com/quic-go/quic-go/internal/utils" ) -func setDF(rawConn syscall.RawConn) error { +func setDF(rawConn syscall.RawConn) (bool, error) { // Enabling IP_MTU_DISCOVER will force the kernel to return "sendto: message too long" // and the datagram will not be fragmented var errDFIPv4, errDFIPv6 error @@ -19,7 +19,7 @@ func setDF(rawConn syscall.RawConn) error { errDFIPv4 = unix.SetsockoptInt(int(fd), unix.IPPROTO_IP, unix.IP_MTU_DISCOVER, unix.IP_PMTUDISC_DO) errDFIPv6 = unix.SetsockoptInt(int(fd), unix.IPPROTO_IPV6, unix.IPV6_MTU_DISCOVER, unix.IPV6_PMTUDISC_DO) }); err != nil { - return err + return false, err } switch { case errDFIPv4 == nil && errDFIPv6 == nil: @@ -29,9 +29,9 @@ func setDF(rawConn syscall.RawConn) error { case errDFIPv4 != nil && errDFIPv6 == nil: utils.DefaultLogger.Debugf("Setting DF for IPv6.") case errDFIPv4 != nil && errDFIPv6 != nil: - return errors.New("setting DF failed for both IPv4 and IPv6") + return false, errors.New("setting DF failed for both IPv4 and IPv6") } - return nil + return true, nil } func isMsgSizeErr(err error) bool { diff --git a/sys_conn_df_windows.go b/sys_conn_df_windows.go index 9855e8de..44486ede 100644 --- a/sys_conn_df_windows.go +++ b/sys_conn_df_windows.go @@ -19,13 +19,13 @@ const ( IPV6_DONTFRAG = 14 ) -func setDF(rawConn syscall.RawConn) error { +func setDF(rawConn syscall.RawConn) (bool, error) { var errDFIPv4, errDFIPv6 error if err := rawConn.Control(func(fd uintptr) { 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 err + return false, err } switch { case errDFIPv4 == nil && errDFIPv6 == nil: @@ -35,9 +35,9 @@ func setDF(rawConn syscall.RawConn) error { case errDFIPv4 != nil && errDFIPv6 == nil: utils.DefaultLogger.Debugf("Setting DF for IPv6.") case errDFIPv4 != nil && errDFIPv6 != nil: - return errors.New("setting DF failed for both IPv4 and IPv6") + return false, errors.New("setting DF failed for both IPv4 and IPv6") } - return nil + return true, nil } func isMsgSizeErr(err error) bool { diff --git a/sys_conn_no_oob.go b/sys_conn_no_oob.go index c5c7f486..997775c9 100644 --- a/sys_conn_no_oob.go +++ b/sys_conn_no_oob.go @@ -4,8 +4,8 @@ package quic import "net" -func newConn(c net.PacketConn) (rawConn, error) { - return &basicConn{PacketConn: c}, nil +func newConn(c net.PacketConn, supportsDF bool) (rawConn, error) { + return &basicConn{PacketConn: c, supportsDF: supportsDF}, nil } func inspectReadBuffer(any) (int, error) { return 0, nil } diff --git a/sys_conn_oob.go b/sys_conn_oob.go index 42e5f53b..4f8963ed 100644 --- a/sys_conn_oob.go +++ b/sys_conn_oob.go @@ -61,11 +61,13 @@ type oobConn struct { // Packets received from the kernel, but not yet returned by ReadPacket(). messages []ipv4.Message buffers [batchSize]*packetBuffer + + supportsDF bool } var _ rawConn = &oobConn{} -func newConn(c OOBCapablePacketConn) (*oobConn, error) { +func newConn(c OOBCapablePacketConn, supportsDF bool) (*oobConn, error) { rawConn, err := c.SyscallConn() if err != nil { return nil, err @@ -132,6 +134,7 @@ func newConn(c OOBCapablePacketConn) (*oobConn, error) { batchConn: bc, messages: msgs, readPos: batchSize, + supportsDF: supportsDF, } for i := 0; i < batchSize; i++ { oobConn.messages[i].OOB = make([]byte, oobBufferSize) @@ -234,6 +237,10 @@ func (c *oobConn) WritePacket(b []byte, addr net.Addr, oob []byte) (n int, err e return n, err } +func (c *oobConn) capabilities() connCapabilities { + return connCapabilities{DF: c.supportsDF} +} + func (info *packetInfo) OOB() []byte { if info == nil { return nil diff --git a/sys_conn_oob_test.go b/sys_conn_oob_test.go index 5df33f48..eab73df4 100644 --- a/sys_conn_oob_test.go +++ b/sys_conn_oob_test.go @@ -24,8 +24,9 @@ var _ = Describe("OOB Conn Test", func() { Expect(err).ToNot(HaveOccurred()) udpConn, err := net.ListenUDP(network, addr) Expect(err).ToNot(HaveOccurred()) - oobConn, err := newConn(udpConn) + oobConn, err := newConn(udpConn, true) Expect(err).ToNot(HaveOccurred()) + Expect(oobConn.capabilities().DF).To(BeTrue()) packetChan := make(chan *receivedPacket) go func() { @@ -228,7 +229,7 @@ var _ = Describe("OOB Conn Test", func() { Expect(err).ToNot(HaveOccurred()) udpConn, err := net.ListenUDP("udp", addr) Expect(err).ToNot(HaveOccurred()) - oobConn, err := newConn(udpConn) + oobConn, err := newConn(udpConn, true) Expect(err).ToNot(HaveOccurred()) oobConn.batchConn = batchConn diff --git a/sys_conn_windows.go b/sys_conn_windows.go index 580d18a5..8a583fda 100644 --- a/sys_conn_windows.go +++ b/sys_conn_windows.go @@ -8,8 +8,8 @@ import ( "golang.org/x/sys/windows" ) -func newConn(c OOBCapablePacketConn) (rawConn, error) { - return &basicConn{PacketConn: c}, nil +func newConn(c OOBCapablePacketConn, supportsDF bool) (rawConn, error) { + return &basicConn{PacketConn: c, supportsDF: supportsDF}, nil } func inspectReadBuffer(c syscall.RawConn) (int, error) { diff --git a/sys_conn_windows_test.go b/sys_conn_windows_test.go index 6bc0ec26..5da4a402 100644 --- a/sys_conn_windows_test.go +++ b/sys_conn_windows_test.go @@ -15,9 +15,10 @@ var _ = Describe("Windows Conn Test", func() { Expect(err).ToNot(HaveOccurred()) udpConn, err := net.ListenUDP("udp4", addr) Expect(err).ToNot(HaveOccurred()) - conn, err := newConn(udpConn) + conn, err := newConn(udpConn, true) Expect(err).ToNot(HaveOccurred()) Expect(conn.Close()).To(Succeed()) + Expect(conn.capabilities().DF).To(BeTrue()) }) It("works on IPv6", func() { @@ -25,8 +26,9 @@ var _ = Describe("Windows Conn Test", func() { Expect(err).ToNot(HaveOccurred()) udpConn, err := net.ListenUDP("udp6", addr) Expect(err).ToNot(HaveOccurred()) - conn, err := newConn(udpConn) + conn, err := newConn(udpConn, false) Expect(err).ToNot(HaveOccurred()) Expect(conn.Close()).To(Succeed()) + Expect(conn.capabilities().DF).To(BeFalse()) }) })