wire: remove FrameParser interface, expose FrameParser struct (quic-g…

…o#4284) Instead, expose the FrameParser struct. This allows us to embed it directly into the connection struct, avoiding a pointer indirection.
mgjeong · Feb 13, 2024 · d336946 · d336946
1 parent 7edd0ad
commit d336946
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 19 deletions.
diff --git a/connection.go b/connection.go
@@ -454,7 +454,7 @@ func (s *connection) preSetup() {
 	s.handshakeStream = newCryptoStream()
 	s.sendQueue = newSendQueue(s.conn)
 	s.retransmissionQueue = newRetransmissionQueue()
-	s.frameParser = wire.NewFrameParser(s.config.EnableDatagrams)
+	s.frameParser = *wire.NewFrameParser(s.config.EnableDatagrams)
 	s.rttStats = &utils.RTTStats{}
 	s.connFlowController = flowcontrol.NewConnectionFlowController(
 		protocol.ByteCount(s.config.InitialConnectionReceiveWindow),

diff --git a/internal/wire/frame_parser.go b/internal/wire/frame_parser.go
@@ -36,7 +36,8 @@ const (
 	handshakeDoneFrameType      = 0x1e
 )
 
-type frameParser struct {
+// The FrameParser parses QUIC frames, one by one.
+type FrameParser struct {
 	r bytes.Reader // cached bytes.Reader, so we don't have to repeatedly allocate them
 
 	ackDelayExponent  uint8
@@ -47,11 +48,9 @@ type frameParser struct {
 	ackFrame *AckFrame
 }
 
-var _ FrameParser = &frameParser{}
-
 // NewFrameParser creates a new frame parser.
-func NewFrameParser(supportsDatagrams bool) *frameParser {
-	return &frameParser{
+func NewFrameParser(supportsDatagrams bool) *FrameParser {
+	return &FrameParser{
 		r:                 *bytes.NewReader(nil),
 		supportsDatagrams: supportsDatagrams,
 		ackFrame:          &AckFrame{},
@@ -60,7 +59,7 @@ func NewFrameParser(supportsDatagrams bool) *frameParser {
 
 // ParseNext parses the next frame.
 // It skips PADDING frames.
-func (p *frameParser) ParseNext(data []byte, encLevel protocol.EncryptionLevel, v protocol.VersionNumber) (int, Frame, error) {
+func (p *FrameParser) ParseNext(data []byte, encLevel protocol.EncryptionLevel, v protocol.VersionNumber) (int, Frame, error) {
 	startLen := len(data)
 	p.r.Reset(data)
 	frame, err := p.parseNext(&p.r, encLevel, v)
@@ -69,7 +68,7 @@ func (p *frameParser) ParseNext(data []byte, encLevel protocol.EncryptionLevel,
 	return n, frame, err
 }
 
-func (p *frameParser) parseNext(r *bytes.Reader, encLevel protocol.EncryptionLevel, v protocol.VersionNumber) (Frame, error) {
+func (p *FrameParser) parseNext(r *bytes.Reader, encLevel protocol.EncryptionLevel, v protocol.VersionNumber) (Frame, error) {
 	for r.Len() != 0 {
 		typ, err := quicvarint.Read(r)
 		if err != nil {
@@ -95,7 +94,7 @@ func (p *frameParser) parseNext(r *bytes.Reader, encLevel protocol.EncryptionLev
 	return nil, nil
 }
 
-func (p *frameParser) parseFrame(r *bytes.Reader, typ uint64, encLevel protocol.EncryptionLevel, v protocol.VersionNumber) (Frame, error) {
+func (p *FrameParser) parseFrame(r *bytes.Reader, typ uint64, encLevel protocol.EncryptionLevel, v protocol.VersionNumber) (Frame, error) {
 	var frame Frame
 	var err error
 	if typ&0xf8 == 0x8 {
@@ -163,7 +162,7 @@ func (p *frameParser) parseFrame(r *bytes.Reader, typ uint64, encLevel protocol.
 	return frame, nil
 }
 
-func (p *frameParser) isAllowedAtEncLevel(f Frame, encLevel protocol.EncryptionLevel) bool {
+func (p *FrameParser) isAllowedAtEncLevel(f Frame, encLevel protocol.EncryptionLevel) bool {
 	switch encLevel {
 	case protocol.EncryptionInitial, protocol.EncryptionHandshake:
 		switch f.(type) {
@@ -186,6 +185,8 @@ func (p *frameParser) isAllowedAtEncLevel(f Frame, encLevel protocol.EncryptionL
 	}
 }
 
-func (p *frameParser) SetAckDelayExponent(exp uint8) {
+// SetAckDelayExponent sets the acknowledgment delay exponent (sent in the transport parameters).
+// This value is used to scale the ACK Delay field in the ACK frame.
+func (p *FrameParser) SetAckDelayExponent(exp uint8) {
 	p.ackDelayExponent = exp
 }
diff --git a/internal/wire/frame_parser_test.go b/internal/wire/frame_parser_test.go
@@ -14,7 +14,7 @@ var _ = Describe("Frame parsing", func() {
 	var parser FrameParser
 
 	BeforeEach(func() {
-		parser = NewFrameParser(true)
+		parser = *NewFrameParser(true)
 	})
 
 	It("returns nil if there's nothing more to read", func() {
@@ -315,7 +315,7 @@ var _ = Describe("Frame parsing", func() {
 	})
 
 	It("errors when DATAGRAM frames are not supported", func() {
-		parser = NewFrameParser(false)
+		parser = *NewFrameParser(false)
 		f := &DatagramFrame{Data: []byte("foobar")}
 		b, err := f.Append(nil, protocol.Version1)
 		Expect(err).ToNot(HaveOccurred())

diff --git a/internal/wire/interface.go b/internal/wire/interface.go
@@ -9,9 +9,3 @@ type Frame interface {
 	Append(b []byte, version protocol.VersionNumber) ([]byte, error)
 	Length(version protocol.VersionNumber) protocol.ByteCount
 }
-
-// A FrameParser parses QUIC frames, one by one.
-type FrameParser interface {
-	ParseNext([]byte, protocol.EncryptionLevel, protocol.VersionNumber) (int, Frame, error)
-	SetAckDelayExponent(uint8)
-}