From b82f062c4bc1abcfe993e3750d64c4bdd4a14f87 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 29 Aug 2023 15:35:30 -0700 Subject: [PATCH] quic: include ignored frames in test log output When looking at a test log, it's a bit confusing to have some of the frames silently omitted. Print ignored frames. Unfortunately, this means we need to do the actual ignoring of frames after printing the packet. We specify frames to ignore by the frame number, but after parsing we don't have a simple way to map from the debugFrame type back to the number. Add a big, ugly mapping function to do this; it's clunky, but isolated to one function in tests. For golang/go#58547 Change-Id: I242f5511dc16be2350fa49030af38588fe92a988 Reviewed-on: https://go-review.googlesource.com/c/net/+/524295 LUCI-TryBot-Result: Go LUCI Reviewed-by: Jonathan Amsterdam --- internal/quic/conn_test.go | 77 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 3 deletions(-) diff --git a/internal/quic/conn_test.go b/internal/quic/conn_test.go index 8ebe49e0e5..d8c44558dc 100644 --- a/internal/quic/conn_test.go +++ b/internal/quic/conn_test.go @@ -431,7 +431,80 @@ func (tc *testConn) readDatagram() *testDatagram { buf := tc.sentDatagrams[0] tc.sentDatagrams = tc.sentDatagrams[1:] d := tc.parseTestDatagram(buf) + // Log the datagram before removing ignored frames. + // When things go wrong, it's useful to see all the frames. tc.logDatagram("-> conn under test sends", d) + typeForFrame := func(f debugFrame) byte { + // This is very clunky, and points at a problem + // in how we specify what frames to ignore in tests. + // + // We mark frames to ignore using the frame type, + // but we've got a debugFrame data structure here. + // Perhaps we should be ignoring frames by debugFrame + // type instead: tc.ignoreFrame[debugFrameAck](). + switch f := f.(type) { + case debugFramePadding: + return frameTypePadding + case debugFramePing: + return frameTypePing + case debugFrameAck: + return frameTypeAck + case debugFrameResetStream: + return frameTypeResetStream + case debugFrameStopSending: + return frameTypeStopSending + case debugFrameCrypto: + return frameTypeCrypto + case debugFrameNewToken: + return frameTypeNewToken + case debugFrameStream: + return frameTypeStreamBase + case debugFrameMaxData: + return frameTypeMaxData + case debugFrameMaxStreamData: + return frameTypeMaxStreamData + case debugFrameMaxStreams: + if f.streamType == bidiStream { + return frameTypeMaxStreamsBidi + } else { + return frameTypeMaxStreamsUni + } + case debugFrameDataBlocked: + return frameTypeDataBlocked + case debugFrameStreamDataBlocked: + return frameTypeStreamDataBlocked + case debugFrameStreamsBlocked: + if f.streamType == bidiStream { + return frameTypeStreamsBlockedBidi + } else { + return frameTypeStreamsBlockedUni + } + case debugFrameNewConnectionID: + return frameTypeNewConnectionID + case debugFrameRetireConnectionID: + return frameTypeRetireConnectionID + case debugFramePathChallenge: + return frameTypePathChallenge + case debugFramePathResponse: + return frameTypePathResponse + case debugFrameConnectionCloseTransport: + return frameTypeConnectionCloseTransport + case debugFrameConnectionCloseApplication: + return frameTypeConnectionCloseApplication + case debugFrameHandshakeDone: + return frameTypeHandshakeDone + } + panic(fmt.Errorf("unhandled frame type %T", f)) + } + for _, p := range d.packets { + var frames []debugFrame + for _, f := range p.frames { + if !tc.ignoreFrames[typeForFrame(f)] { + frames = append(frames, f) + } + } + p.frames = frames + } return d } @@ -632,9 +705,7 @@ func (tc *testConn) parseTestFrames(payload []byte) ([]debugFrame, error) { if n < 0 { return nil, errors.New("error parsing frames") } - if !tc.ignoreFrames[payload[0]] { - frames = append(frames, f) - } + frames = append(frames, f) payload = payload[n:] } return frames, nil