From 3b5950a1cea563ef974b8af784a8f8325b843c97 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 13 May 2023 15:19:58 +0300 Subject: [PATCH 1/2] use a netip.Addr instead of a net.IP in the packetInfo struct --- send_conn.go | 2 +- sys_conn_no_oob.go | 7 +++++-- sys_conn_oob.go | 44 +++++++++++++++++++++----------------------- sys_conn_oob_test.go | 12 +++++++----- sys_conn_windows.go | 4 ++-- 5 files changed, 36 insertions(+), 33 deletions(-) diff --git a/send_conn.go b/send_conn.go index fab367470..f2bc0bd8e 100644 --- a/send_conn.go +++ b/send_conn.go @@ -60,7 +60,7 @@ func (c *sconn) LocalAddr() net.Addr { if c.info != nil { if udpAddr, ok := addr.(*net.UDPAddr); ok { addrCopy := *udpAddr - addrCopy.IP = c.info.addr + addrCopy.IP = c.info.addr.AsSlice() addr = &addrCopy } } diff --git a/sys_conn_no_oob.go b/sys_conn_no_oob.go index e99f81bdc..2a1f807ef 100644 --- a/sys_conn_no_oob.go +++ b/sys_conn_no_oob.go @@ -2,7 +2,10 @@ package quic -import "net" +import ( + "net" + "net/netip" +) func newConn(c net.PacketConn, supportsDF bool) (*basicConn, error) { return &basicConn{PacketConn: c, supportsDF: supportsDF}, nil @@ -12,7 +15,7 @@ func inspectReadBuffer(any) (int, error) { return 0, nil } func inspectWriteBuffer(any) (int, error) { return 0, nil } type packetInfo struct { - addr net.IP + addr netip.Addr } func (i *packetInfo) OOB() []byte { return nil } diff --git a/sys_conn_oob.go b/sys_conn_oob.go index 5e6213b19..5528ef812 100644 --- a/sys_conn_oob.go +++ b/sys_conn_oob.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net" + "net/netip" "syscall" "time" @@ -173,8 +174,7 @@ func (c *oobConn) ReadPacket() (receivedPacket, error) { data := msg.OOB[:msg.NN] var ecn protocol.ECN - var destIP net.IP - var ifIndex uint32 + var info *packetInfo for len(data) > 0 { hdr, body, remainder, err := unix.ParseOneSocketControlMessage(data) if err != nil { @@ -191,15 +191,16 @@ func (c *oobConn) ReadPacket() (receivedPacket, error) { // struct in_addr ipi_addr; /* Header Destination // address */ // }; - ip := make([]byte, 4) + info = &packetInfo{} + var ip [4]byte if len(body) == 12 { - ifIndex = binary.LittleEndian.Uint32(body) - copy(ip, body[8:12]) + copy(ip[:], body[8:12]) + info.ifIndex = binary.LittleEndian.Uint32(body) } else if len(body) == 4 { // FreeBSD - copy(ip, body) + copy(ip[:], body) } - destIP = net.IP(ip) + info.addr = netip.AddrFrom4(ip) } } if hdr.Level == unix.IPPROTO_IPV6 { @@ -212,22 +213,17 @@ func (c *oobConn) ReadPacket() (receivedPacket, error) { // unsigned int ipi6_ifindex; /* send/recv interface index */ // }; if len(body) == 20 { - ip := make([]byte, 16) - copy(ip, body[:16]) - destIP = net.IP(ip) - ifIndex = binary.LittleEndian.Uint32(body[16:]) + var ip [16]byte + copy(ip[:], body[:16]) + info = &packetInfo{ + addr: netip.AddrFrom16(ip), + ifIndex: binary.LittleEndian.Uint32(body[16:]), + } } } } data = remainder } - var info *packetInfo - if destIP != nil { - info = &packetInfo{ - addr: destIP, - ifIndex: ifIndex, - } - } return receivedPacket{ remoteAddr: msg.Addr, rcvTime: time.Now(), @@ -265,7 +261,7 @@ func (c *oobConn) capabilities() connCapabilities { } type packetInfo struct { - addr net.IP + addr netip.Addr ifIndex uint32 } @@ -273,24 +269,26 @@ func (info *packetInfo) OOB() []byte { if info == nil { return nil } - if ip4 := info.addr.To4(); ip4 != nil { + if info.addr.Is4() { + ip := info.addr.As4() // struct in_pktinfo { // unsigned int ipi_ifindex; /* Interface index */ // struct in_addr ipi_spec_dst; /* Local address */ // struct in_addr ipi_addr; /* Header Destination address */ // }; cm := ipv4.ControlMessage{ - Src: ip4, + Src: ip[:], IfIndex: int(info.ifIndex), } return cm.Marshal() - } else if len(info.addr) == 16 { + } else if info.addr.Is6() { + ip := info.addr.As16() // struct in6_pktinfo { // struct in6_addr ipi6_addr; /* src/dst IPv6 address */ // unsigned int ipi6_ifindex; /* send/recv interface index */ // }; cm := ipv6.ControlMessage{ - Src: info.addr, + Src: ip[:], IfIndex: int(info.ifIndex), } return cm.Marshal() diff --git a/sys_conn_oob_test.go b/sys_conn_oob_test.go index 57df623c6..f6f864f7c 100644 --- a/sys_conn_oob_test.go +++ b/sys_conn_oob_test.go @@ -155,7 +155,7 @@ var _ = Describe("OOB Conn Test", func() { Expect(p.data).To(Equal([]byte("foobar"))) Expect(p.remoteAddr).To(Equal(sentFrom)) Expect(p.info).To(Not(BeNil())) - Expect(p.info.addr.To4()).To(Equal(ip)) + Expect(net.IP(p.info.addr.AsSlice())).To(Equal(ip)) }) It("reads packet info on IPv6", func() { @@ -173,7 +173,7 @@ var _ = Describe("OOB Conn Test", func() { Expect(p.data).To(Equal([]byte("foobar"))) Expect(p.remoteAddr).To(Equal(sentFrom)) Expect(p.info).To(Not(BeNil())) - Expect(p.info.addr).To(Equal(ip)) + Expect(net.IP(p.info.addr.AsSlice())).To(Equal(ip)) }) It("reads packet info on a connection that supports both IPv4 and IPv6", func() { @@ -182,14 +182,16 @@ var _ = Describe("OOB Conn Test", func() { port := conn.LocalAddr().(*net.UDPAddr).Port // IPv4 - ip4 := net.ParseIP("127.0.0.1").To4() + ip4 := net.ParseIP("127.0.0.1") sendPacket("udp4", &net.UDPAddr{IP: ip4, Port: port}) var p receivedPacket Eventually(packetChan).Should(Receive(&p)) Expect(utils.IsIPv4(p.remoteAddr.(*net.UDPAddr).IP)).To(BeTrue()) Expect(p.info).To(Not(BeNil())) - Expect(p.info.addr.To4()).To(Equal(ip4)) + Expect(p.info.addr.Is4In6() || p.info.addr.Is4()).To(BeTrue()) + ip := p.info.addr.As4() + Expect(net.IP(ip[:])).To(Equal(ip4.To4())) // IPv6 ip6 := net.ParseIP("::1") @@ -198,7 +200,7 @@ var _ = Describe("OOB Conn Test", func() { Eventually(packetChan).Should(Receive(&p)) Expect(utils.IsIPv4(p.remoteAddr.(*net.UDPAddr).IP)).To(BeFalse()) Expect(p.info).To(Not(BeNil())) - Expect(p.info.addr).To(Equal(ip6)) + Expect(net.IP(p.info.addr.AsSlice())).To(Equal(ip6)) }) }) diff --git a/sys_conn_windows.go b/sys_conn_windows.go index 8687f7501..b9c1cbc86 100644 --- a/sys_conn_windows.go +++ b/sys_conn_windows.go @@ -3,7 +3,7 @@ package quic import ( - "net" + "net/netip" "syscall" "golang.org/x/sys/windows" @@ -36,7 +36,7 @@ func inspectWriteBuffer(c syscall.RawConn) (int, error) { } type packetInfo struct { - addr net.IP + addr netip.Addr } func (i *packetInfo) OOB() []byte { return nil } From edaeed0107fea11364cbfa68577402443a1a6f8b Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 13 May 2023 15:35:41 +0300 Subject: [PATCH 2/2] embed the packetInfo in the receivedPacket struct This avoid allocating the packetInfo struct when receiving a packet. --- closed_conn.go | 4 ++-- closed_conn_test.go | 2 +- connection.go | 2 +- packet_handler_map.go | 4 ++-- send_conn.go | 18 +++++++++++++++--- send_conn_test.go | 2 +- server.go | 8 ++++---- sys_conn_oob.go | 32 +++++++++++++------------------- sys_conn_oob_test.go | 2 +- transport.go | 4 ++-- 10 files changed, 42 insertions(+), 36 deletions(-) diff --git a/closed_conn.go b/closed_conn.go index 901bb8aee..0c988b53e 100644 --- a/closed_conn.go +++ b/closed_conn.go @@ -16,13 +16,13 @@ type closedLocalConn struct { perspective protocol.Perspective logger utils.Logger - sendPacket func(net.Addr, *packetInfo) + sendPacket func(net.Addr, packetInfo) } var _ packetHandler = &closedLocalConn{} // newClosedLocalConn creates a new closedLocalConn and runs it. -func newClosedLocalConn(sendPacket func(net.Addr, *packetInfo), pers protocol.Perspective, logger utils.Logger) packetHandler { +func newClosedLocalConn(sendPacket func(net.Addr, packetInfo), pers protocol.Perspective, logger utils.Logger) packetHandler { return &closedLocalConn{ sendPacket: sendPacket, perspective: pers, diff --git a/closed_conn_test.go b/closed_conn_test.go index 21fddab4b..ac9da31e6 100644 --- a/closed_conn_test.go +++ b/closed_conn_test.go @@ -21,7 +21,7 @@ var _ = Describe("Closed local connection", func() { It("repeats the packet containing the CONNECTION_CLOSE frame", func() { written := make(chan net.Addr, 1) conn := newClosedLocalConn( - func(addr net.Addr, _ *packetInfo) { written <- addr }, + func(addr net.Addr, _ packetInfo) { written <- addr }, protocol.PerspectiveClient, utils.DefaultLogger, ) diff --git a/connection.go b/connection.go index e081e45fc..c4668503c 100644 --- a/connection.go +++ b/connection.go @@ -70,7 +70,7 @@ type receivedPacket struct { ecn protocol.ECN - info *packetInfo + info packetInfo // only valid if the contained IP address is valid } func (p *receivedPacket) Size() protocol.ByteCount { return protocol.ByteCount(len(p.data)) } diff --git a/packet_handler_map.go b/packet_handler_map.go index 47f0dcc22..2a16773c8 100644 --- a/packet_handler_map.go +++ b/packet_handler_map.go @@ -39,7 +39,7 @@ type rawConn interface { type closePacket struct { payload []byte addr net.Addr - info *packetInfo + info packetInfo } type unknownPacketHandler interface { @@ -177,7 +177,7 @@ func (h *packetHandlerMap) ReplaceWithClosed(ids []protocol.ConnectionID, pers p var handler packetHandler if connClosePacket != nil { handler = newClosedLocalConn( - func(addr net.Addr, info *packetInfo) { + func(addr net.Addr, info packetInfo) { h.enqueueClosePacket(closePacket{payload: connClosePacket, addr: addr, info: info}) }, pers, diff --git a/send_conn.go b/send_conn.go index f2bc0bd8e..4e7007fad 100644 --- a/send_conn.go +++ b/send_conn.go @@ -21,13 +21,25 @@ type sconn struct { rawConn remoteAddr net.Addr - info *packetInfo + info packetInfo oob []byte } var _ sendConn = &sconn{} -func newSendConn(c rawConn, remote net.Addr, info *packetInfo) *sconn { +func newSendConn(c rawConn, remote net.Addr) *sconn { + sc := &sconn{ + rawConn: c, + remoteAddr: remote, + } + if c.capabilities().GSO { + // add 32 bytes, so we can add the UDP_SEGMENT msg + sc.oob = make([]byte, 0, 32) + } + return sc +} + +func newSendConnWithPacketInfo(c rawConn, remote net.Addr, info packetInfo) *sconn { oob := info.OOB() if c.capabilities().GSO { // add 32 bytes, so we can add the UDP_SEGMENT msg @@ -57,7 +69,7 @@ func (c *sconn) RemoteAddr() net.Addr { func (c *sconn) LocalAddr() net.Addr { addr := c.rawConn.LocalAddr() - if c.info != nil { + if c.info.addr.IsValid() { if udpAddr, ok := addr.(*net.UDPAddr); ok { addrCopy := *udpAddr addrCopy.IP = c.info.addr.AsSlice() diff --git a/send_conn_test.go b/send_conn_test.go index 0b5cc6214..56fe92366 100644 --- a/send_conn_test.go +++ b/send_conn_test.go @@ -19,7 +19,7 @@ var _ = Describe("Connection (for sending packets)", func() { packetConn = NewMockPacketConn(mockCtrl) rawConn, err := wrapConn(packetConn) Expect(err).ToNot(HaveOccurred()) - c = newSendConn(rawConn, addr, nil) + c = newSendConnWithPacketInfo(rawConn, addr, packetInfo{}) }) It("writes", func() { diff --git a/server.go b/server.go index 8337a978f..0f8219e3a 100644 --- a/server.go +++ b/server.go @@ -632,7 +632,7 @@ func (s *baseServer) handleInitialImpl(p receivedPacket, hdr *wire.Header) error tracer = config.Tracer(context.WithValue(context.Background(), ConnectionTracingKey, tracingID), protocol.PerspectiveServer, connID) } conn = s.newConn( - newSendConn(s.conn, p.remoteAddr, p.info), + newSendConnWithPacketInfo(s.conn, p.remoteAddr, p.info), s.connHandler, origDestConnID, retrySrcConnID, @@ -706,7 +706,7 @@ func (s *baseServer) handleNewConn(conn quicConn) { } } -func (s *baseServer) sendRetry(remoteAddr net.Addr, hdr *wire.Header, info *packetInfo) error { +func (s *baseServer) sendRetry(remoteAddr net.Addr, hdr *wire.Header, info packetInfo) error { // Log the Initial packet now. // If no Retry is sent, the packet will be logged by the connection. (&wire.ExtendedHeader{Header: *hdr}).Log(s.logger) @@ -795,13 +795,13 @@ func (s *baseServer) maybeSendInvalidToken(p receivedPacket) { } } -func (s *baseServer) sendConnectionRefused(remoteAddr net.Addr, hdr *wire.Header, info *packetInfo) error { +func (s *baseServer) sendConnectionRefused(remoteAddr net.Addr, hdr *wire.Header, info packetInfo) error { sealer, _ := handshake.NewInitialAEAD(hdr.DestConnectionID, protocol.PerspectiveServer, hdr.Version) return s.sendError(remoteAddr, hdr, sealer, qerr.ConnectionRefused, info) } // sendError sends the error as a response to the packet received with header hdr -func (s *baseServer) sendError(remoteAddr net.Addr, hdr *wire.Header, sealer handshake.LongHeaderSealer, errorCode qerr.TransportErrorCode, info *packetInfo) error { +func (s *baseServer) sendError(remoteAddr net.Addr, hdr *wire.Header, sealer handshake.LongHeaderSealer, errorCode qerr.TransportErrorCode, info packetInfo) error { b := getPacketBuffer() defer b.Release() diff --git a/sys_conn_oob.go b/sys_conn_oob.go index 5528ef812..fc28f94b7 100644 --- a/sys_conn_oob.go +++ b/sys_conn_oob.go @@ -173,8 +173,12 @@ func (c *oobConn) ReadPacket() (receivedPacket, error) { c.readPos++ data := msg.OOB[:msg.NN] - var ecn protocol.ECN - var info *packetInfo + p := receivedPacket{ + remoteAddr: msg.Addr, + rcvTime: time.Now(), + data: msg.Buffers[0][:msg.N], + buffer: buffer, + } for len(data) > 0 { hdr, body, remainder, err := unix.ParseOneSocketControlMessage(data) if err != nil { @@ -183,7 +187,7 @@ func (c *oobConn) ReadPacket() (receivedPacket, error) { if hdr.Level == unix.IPPROTO_IP { switch hdr.Type { case msgTypeIPTOS: - ecn = protocol.ECN(body[0] & ecnMask) + p.ecn = protocol.ECN(body[0] & ecnMask) case msgTypeIPv4PKTINFO: // struct in_pktinfo { // unsigned int ipi_ifindex; /* Interface index */ @@ -191,22 +195,21 @@ func (c *oobConn) ReadPacket() (receivedPacket, error) { // struct in_addr ipi_addr; /* Header Destination // address */ // }; - info = &packetInfo{} var ip [4]byte if len(body) == 12 { copy(ip[:], body[8:12]) - info.ifIndex = binary.LittleEndian.Uint32(body) + p.info.ifIndex = binary.LittleEndian.Uint32(body) } else if len(body) == 4 { // FreeBSD copy(ip[:], body) } - info.addr = netip.AddrFrom4(ip) + p.info.addr = netip.AddrFrom4(ip) } } if hdr.Level == unix.IPPROTO_IPV6 { switch hdr.Type { case unix.IPV6_TCLASS: - ecn = protocol.ECN(body[0] & ecnMask) + p.ecn = protocol.ECN(body[0] & ecnMask) case msgTypeIPv6PKTINFO: // struct in6_pktinfo { // struct in6_addr ipi6_addr; /* src/dst IPv6 address */ @@ -215,23 +218,14 @@ func (c *oobConn) ReadPacket() (receivedPacket, error) { if len(body) == 20 { var ip [16]byte copy(ip[:], body[:16]) - info = &packetInfo{ - addr: netip.AddrFrom16(ip), - ifIndex: binary.LittleEndian.Uint32(body[16:]), - } + p.info.addr = netip.AddrFrom16(ip) + p.info.ifIndex = binary.LittleEndian.Uint32(body[16:]) } } } data = remainder } - return receivedPacket{ - remoteAddr: msg.Addr, - rcvTime: time.Now(), - data: msg.Buffers[0][:msg.N], - ecn: ecn, - info: info, - buffer: buffer, - }, nil + return p, nil } // WriteTo (re)implements the net.PacketConn method. diff --git a/sys_conn_oob_test.go b/sys_conn_oob_test.go index f6f864f7c..30b333b97 100644 --- a/sys_conn_oob_test.go +++ b/sys_conn_oob_test.go @@ -154,7 +154,7 @@ var _ = Describe("OOB Conn Test", func() { Expect(p.rcvTime).To(BeTemporally("~", time.Now(), scaleDuration(20*time.Millisecond))) Expect(p.data).To(Equal([]byte("foobar"))) Expect(p.remoteAddr).To(Equal(sentFrom)) - Expect(p.info).To(Not(BeNil())) + Expect(p.info.addr.IsValid()).To(BeTrue()) Expect(net.IP(p.info.addr.AsSlice())).To(Equal(ip)) }) diff --git a/transport.go b/transport.go index cc3d294fb..242eedc10 100644 --- a/transport.go +++ b/transport.go @@ -155,7 +155,7 @@ func (t *Transport) Dial(ctx context.Context, addr net.Addr, tlsConf *tls.Config if t.isSingleUse { onClose = func() { t.Close() } } - return dial(ctx, newSendConn(t.conn, addr, nil), t.connIDGenerator, t.handlerMap, tlsConf, conf, onClose, false) + return dial(ctx, newSendConn(t.conn, addr), t.connIDGenerator, t.handlerMap, tlsConf, conf, onClose, false) } // DialEarly dials a new connection, attempting to use 0-RTT if possible. @@ -171,7 +171,7 @@ func (t *Transport) DialEarly(ctx context.Context, addr net.Addr, tlsConf *tls.C if t.isSingleUse { onClose = func() { t.Close() } } - return dial(ctx, newSendConn(t.conn, addr, nil), t.connIDGenerator, t.handlerMap, tlsConf, conf, onClose, true) + return dial(ctx, newSendConn(t.conn, addr), t.connIDGenerator, t.handlerMap, tlsConf, conf, onClose, true) } func (t *Transport) init(isServer bool) error {