Specify subsequent layers as ICMP payloads and add ICMPv6.Pointer

Moves specification of subsequent layers to the Payload field of the ICMP and
ICMPv6 layers.

Removes the need to manually encode type-specific ICMP or ICMPv6 headers, such
as the "pointer" header of parameter problem messages, in the payload. This is
necessary prework to enable matching of more type-specific headers, such as
"ident" of echo requests.

Fixes a bug where the "reserved" header defined by ICMPv6 RFC 4443 was being
incorrectly encoded by ToBytes. This change fixes the implementation by copying
the payload into Payload, instead of the MessageBody.

PiperOrigin-RevId: 376026420

7 files changed, 89 insertions, 107 deletions

diff --git a/test/packetimpact/testbench/layers.go b/test/packetimpact/testbench/layers.go
index 2311f7686..ef8b63db4 100644
--- a/test/packetimpact/testbench/layers.go
+++ b/test/packetimpact/testbench/layers.go
@@ -824,6 +824,7 @@ type ICMPv6 struct {
 	Type     *header.ICMPv6Type
 	Code     *header.ICMPv6Code
 	Checksum *uint16
+	Pointer  *uint32 // Only in Parameter Problem.
 	Payload  []byte
 }
 
@@ -835,7 +836,7 @@ func (l *ICMPv6) String() string {
 
 // ToBytes implements Layer.ToBytes.
 func (l *ICMPv6) ToBytes() ([]byte, error) {
-	b := make([]byte, header.ICMPv6HeaderSize+len(l.Payload))
+	b := make([]byte, header.ICMPv6MinimumSize+len(l.Payload))
 	h := header.ICMPv6(b)
 	if l.Type != nil {
 		h.SetType(*l.Type)
@@ -843,27 +844,30 @@ func (l *ICMPv6) ToBytes() ([]byte, error) {
 	if l.Code != nil {
 		h.SetCode(*l.Code)
 	}
-	if n := copy(h.MessageBody(), l.Payload); n != len(l.Payload) {
+	if n := copy(h.Payload(), l.Payload); n != len(l.Payload) {
 		panic(fmt.Sprintf("copied %d bytes, expected to copy %d bytes", n, len(l.Payload)))
 	}
+	typ := h.Type()
+	switch typ {
+	case header.ICMPv6ParamProblem:
+		if l.Pointer != nil {
+			h.SetTypeSpecific(*l.Pointer)
+		}
+	}
 	if l.Checksum != nil {
 		h.SetChecksum(*l.Checksum)
 	} else {
 		// It is possible that the ICMPv6 header does not follow the IPv6 header
 		// immediately, there could be one or more extension headers in between.
-		// We need to search forward to find the IPv6 header.
-		for prev := l.Prev(); prev != nil; prev = prev.Prev() {
-			if ipv6, ok := prev.(*IPv6); ok {
-				payload, err := payload(l)
-				if err != nil {
-					return nil, err
-				}
+		// We need to search backwards to find the IPv6 header.
+		for layer := l.Prev(); layer != nil; layer = layer.Prev() {
+			if ipv6, ok := layer.(*IPv6); ok {
 				h.SetChecksum(header.ICMPv6Checksum(header.ICMPv6ChecksumParams{
-					Header:      h,
+					Header:      h[:header.ICMPv6PayloadOffset],
 					Src:         *ipv6.SrcAddr,
 					Dst:         *ipv6.DstAddr,
-					PayloadCsum: header.ChecksumVV(payload, 0 /* initial */),
-					PayloadLen:  payload.Size(),
+					PayloadCsum: header.Checksum(l.Payload, 0 /* initial */),
+					PayloadLen:  len(l.Payload),
 				}))
 				break
 			}
@@ -884,20 +888,19 @@ func ICMPv6Code(v header.ICMPv6Code) *header.ICMPv6Code {
 	return &v
 }
 
-// Byte is a helper routine that allocates a new byte value to store
-// v and returns a pointer to it.
-func Byte(v byte) *byte {
-	return &v
-}
-
 // parseICMPv6 parses the bytes assuming that they start with an ICMPv6 header.
 func parseICMPv6(b []byte) (Layer, layerParser) {
 	h := header.ICMPv6(b)
+	msgType := h.Type()
 	icmpv6 := ICMPv6{
-		Type:     ICMPv6Type(h.Type()),
+		Type:     ICMPv6Type(msgType),
 		Code:     ICMPv6Code(h.Code()),
 		Checksum: Uint16(h.Checksum()),
-		Payload:  h.MessageBody(),
+		Payload:  h.Payload(),
+	}
+	switch msgType {
+	case header.ICMPv6ParamProblem:
+		icmpv6.Pointer = Uint32(h.TypeSpecific())
 	}
 	return &icmpv6, nil
 }
@@ -907,7 +910,7 @@ func (l *ICMPv6) match(other Layer) bool {
 }
 
 func (l *ICMPv6) length() int {
-	return header.ICMPv6HeaderSize + len(l.Payload)
+	return header.ICMPv6MinimumSize + len(l.Payload)
 }
 
 // merge overrides the values in l with the values from other but only in fields
@@ -954,8 +957,8 @@ func (l *ICMPv4) ToBytes() ([]byte, error) {
 	if l.Code != nil {
 		h.SetCode(*l.Code)
 	}
-	if copied := copy(h.Payload(), l.Payload); copied != len(l.Payload) {
-		panic(fmt.Sprintf("wrong number of bytes copied into h.Payload(): got = %d, want = %d", len(h.Payload()), len(l.Payload)))
+	if n := copy(h.Payload(), l.Payload); n != len(l.Payload) {
+		panic(fmt.Sprintf("wrong number of bytes copied into h.Payload(): got = %d, want = %d", n, len(l.Payload)))
 	}
 	typ := h.Type()
 	switch typ {
@@ -977,16 +980,7 @@ func (l *ICMPv4) ToBytes() ([]byte, error) {
 	if l.Checksum != nil {
 		h.SetChecksum(*l.Checksum)
 	} else {
-		// Compute the checksum based on the ICMPv4.Payload and also the subsequent
-		// layers.
-		payload, err := payload(l)
-		if err != nil {
-			return nil, err
-		}
-		var vv buffer.VectorisedView
-		vv.AppendView(buffer.View(l.Payload))
-		vv.Append(payload)
-		h.SetChecksum(header.ICMPv4Checksum(h, header.ChecksumVV(vv, 0 /* initial */)))
+		h.SetChecksum(^header.Checksum(h, 0))
 	}
 
 	return h, nil
@@ -1019,7 +1013,7 @@ func (l *ICMPv4) match(other Layer) bool {
 }
 
 func (l *ICMPv4) length() int {
-	return header.ICMPv4MinimumSize
+	return header.ICMPv4MinimumSize + len(l.Payload)
 }
 
 // merge overrides the values in l with the values from other but only in fields
diff --git a/test/packetimpact/testbench/layers_test.go b/test/packetimpact/testbench/layers_test.go
index 614a5de1e..bc96e0c88 100644
--- a/test/packetimpact/testbench/layers_test.go
+++ b/test/packetimpact/testbench/layers_test.go
@@ -596,7 +596,7 @@ func TestIPv6ExtHdrOptions(t *testing.T) {
 					Type:     ICMPv6Type(header.ICMPv6ParamProblem),
 					Code:     ICMPv6Code(header.ICMPv6ErroneousHeader),
 					Checksum: Uint16(0x5f98),
-					Payload:  []byte{0x00, 0x00, 0x00, 0x06},
+					Pointer:  Uint32(6),
 				},
 			},
 		},
diff --git a/test/packetimpact/tests/icmpv6_param_problem_test.go b/test/packetimpact/tests/icmpv6_param_problem_test.go
index 1beccb6cf..fdabcf1ad 100644
--- a/test/packetimpact/tests/icmpv6_param_problem_test.go
+++ b/test/packetimpact/tests/icmpv6_param_problem_test.go
@@ -15,7 +15,6 @@
 package icmpv6_param_problem_test
 
 import (
-	"encoding/binary"
 	"flag"
 	"testing"
 	"time"
@@ -56,13 +55,11 @@ func TestICMPv6ParamProblemTest(t *testing.T) {
 		t.Fatalf("can't convert %s to bytes: %s", ipv6Sent, err)
 	}
 
-	// The problematic field is the NextHeader.
-	b := make([]byte, 4)
-	binary.BigEndian.PutUint32(b, header.IPv6NextHeaderOffset)
-	expectedPayload = append(b, expectedPayload...)
 	expectedICMPv6 := testbench.ICMPv6{
 		Type:    testbench.ICMPv6Type(header.ICMPv6ParamProblem),
 		Payload: expectedPayload,
+		// The problematic field is the NextHeader.
+		Pointer: testbench.Uint32(header.IPv6NextHeaderOffset),
 	}
 
 	paramProblem := testbench.Layers{
diff --git a/test/packetimpact/tests/ipv6_unknown_options_action_test.go b/test/packetimpact/tests/ipv6_unknown_options_action_test.go
index f999d13d2..d762c43a7 100644
--- a/test/packetimpact/tests/ipv6_unknown_options_action_test.go
+++ b/test/packetimpact/tests/ipv6_unknown_options_action_test.go
@@ -15,7 +15,6 @@
 package ipv6_unknown_options_action_test
 
 import (
-	"encoding/binary"
 	"flag"
 	"net"
 	"testing"
@@ -154,23 +153,22 @@ func TestIPv6UnknownOptionAction(t *testing.T) {
 			outgoing := conn.CreateFrame(t, outgoingOverride, tt.mkExtHdr(optionTypeFromAction(tt.action)))
 			conn.SendFrame(t, outgoing)
 			ipv6Sent := outgoing[1:]
-			invokingPacket, err := ipv6Sent.ToBytes()
+			icmpv6Payload, err := ipv6Sent.ToBytes()
 			if err != nil {
 				t.Fatalf("failed to serialize the outgoing packet: %s", err)
 			}
-			icmpv6Payload := make([]byte, 4)
-			// The pointer in the ICMPv6 parameter problem message should point to
-			// the option type of the unknown option. In our test case, it is the
-			// first option in the extension header whose option type is 2 bytes
-			// after the IPv6 header (after NextHeader and ExtHdrLen).
-			binary.BigEndian.PutUint32(icmpv6Payload, header.IPv6MinimumSize+2)
-			icmpv6Payload = append(icmpv6Payload, invokingPacket...)
 			gotICMPv6, err := conn.ExpectFrame(t, testbench.Layers{
 				&testbench.Ether{},
 				&testbench.IPv6{},
 				&testbench.ICMPv6{
-					Type:    testbench.ICMPv6Type(header.ICMPv6ParamProblem),
-					Code:    testbench.ICMPv6Code(header.ICMPv6UnknownOption),
+					Type: testbench.ICMPv6Type(header.ICMPv6ParamProblem),
+					Code: testbench.ICMPv6Code(header.ICMPv6UnknownOption),
+					// The pointer in the ICMPv6 parameter problem message
+					// should point to the option type of the unknown option. In
+					// our test case, it is the first option in the extension
+					// header whose option type is 2 bytes after the IPv6 header
+					// (after NextHeader and ExtHdrLen).
+					Pointer: testbench.Uint32(header.IPv6MinimumSize + 2),
 					Payload: icmpv6Payload,
 				},
 			}, time.Second)
diff --git a/test/packetimpact/tests/tcp_connect_icmp_error_test.go b/test/packetimpact/tests/tcp_connect_icmp_error_test.go
index 79bfe9eb7..3b4c4cd63 100644
--- a/test/packetimpact/tests/tcp_connect_icmp_error_test.go
+++ b/test/packetimpact/tests/tcp_connect_icmp_error_test.go
@@ -33,17 +33,18 @@ func init() {
 func sendICMPError(t *testing.T, conn *testbench.TCPIPv4, tcp *testbench.TCP) {
 	t.Helper()
 
-	layers := conn.CreateFrame(t, nil)
-	layers = layers[:len(layers)-1]
-	ip, ok := tcp.Prev().(*testbench.IPv4)
-	if !ok {
-		t.Fatalf("expected %s to be IPv4", tcp.Prev())
+	icmpPayload := testbench.Layers{tcp.Prev(), tcp}
+	bytes, err := icmpPayload.ToBytes()
+	if err != nil {
+		t.Fatalf("got icmpPayload.ToBytes() = (_, %s), want = (_, nil)", err)
 	}
-	icmpErr := &testbench.ICMPv4{
-		Type: testbench.ICMPv4Type(header.ICMPv4DstUnreachable),
-		Code: testbench.ICMPv4Code(header.ICMPv4HostUnreachable)}
 
-	layers = append(layers, icmpErr, ip, tcp)
+	layers := conn.CreateFrame(t, nil)
+	layers[len(layers)-1] = &testbench.ICMPv4{
+		Type:    testbench.ICMPv4Type(header.ICMPv4DstUnreachable),
+		Code:    testbench.ICMPv4Code(header.ICMPv4HostUnreachable),
+		Payload: bytes,
+	}
 	conn.SendFrameStateless(t, layers)
 }
 
diff --git a/test/packetimpact/tests/tcp_network_unreachable_test.go b/test/packetimpact/tests/tcp_network_unreachable_test.go
index 5168450ad..60a2dbf3d 100644
--- a/test/packetimpact/tests/tcp_network_unreachable_test.go
+++ b/test/packetimpact/tests/tcp_network_unreachable_test.go
@@ -50,30 +50,24 @@ func TestTCPSynSentUnreachable(t *testing.T) {
 	}
 
 	// Get the SYN.
-	tcpLayers, err := conn.ExpectData(t, &testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn)}, nil, time.Second)
+	tcp, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn)}, time.Second)
 	if err != nil {
 		t.Fatalf("expected SYN: %s", err)
 	}
 
 	// Send a host unreachable message.
-	layers := conn.CreateFrame(t, nil)
-	layers = layers[:len(layers)-1]
-	const ipLayer = 1
-	const tcpLayer = ipLayer + 1
-	ip, ok := tcpLayers[ipLayer].(*testbench.IPv4)
-	if !ok {
-		t.Fatalf("expected %s to be IPv4", tcpLayers[ipLayer])
-	}
-	tcp, ok := tcpLayers[tcpLayer].(*testbench.TCP)
-	if !ok {
-		t.Fatalf("expected %s to be TCP", tcpLayers[tcpLayer])
-	}
-	var icmpv4 testbench.ICMPv4 = testbench.ICMPv4{
-		Type: testbench.ICMPv4Type(header.ICMPv4DstUnreachable),
-		Code: testbench.ICMPv4Code(header.ICMPv4HostUnreachable),
+	icmpPayload := testbench.Layers{tcp.Prev(), tcp}
+	bytes, err := icmpPayload.ToBytes()
+	if err != nil {
+		t.Fatalf("got icmpPayload.ToBytes() = (_, %s), want = (_, nil)", err)
 	}
 
-	layers = append(layers, &icmpv4, ip, tcp)
+	layers := conn.CreateFrame(t, nil)
+	layers[len(layers)-1] = &testbench.ICMPv4{
+		Type:    testbench.ICMPv4Type(header.ICMPv4DstUnreachable),
+		Code:    testbench.ICMPv4Code(header.ICMPv4HostUnreachable),
+		Payload: bytes,
+	}
 	conn.SendFrameStateless(t, layers)
 
 	if err := getConnectError(t, &dut, clientFD); err != unix.EHOSTUNREACH {
@@ -104,31 +98,24 @@ func TestTCPSynSentUnreachable6(t *testing.T) {
 	}
 
 	// Get the SYN.
-	tcpLayers, err := conn.ExpectData(t, &testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn)}, nil, time.Second)
+	tcp, err := conn.Expect(t, &testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn)}, time.Second)
 	if err != nil {
 		t.Fatalf("expected SYN: %s", err)
 	}
 
 	// Send a host unreachable message.
-	layers := conn.CreateFrame(t, nil)
-	layers = layers[:len(layers)-1]
-	const ipLayer = 1
-	const tcpLayer = ipLayer + 1
-	ip, ok := tcpLayers[ipLayer].(*testbench.IPv6)
-	if !ok {
-		t.Fatalf("expected %s to be IPv6", tcpLayers[ipLayer])
-	}
-	tcp, ok := tcpLayers[tcpLayer].(*testbench.TCP)
-	if !ok {
-		t.Fatalf("expected %s to be TCP", tcpLayers[tcpLayer])
+	icmpPayload := testbench.Layers{tcp.Prev(), tcp}
+	bytes, err := icmpPayload.ToBytes()
+	if err != nil {
+		t.Fatalf("got icmpPayload.ToBytes() = (_, %s), want = (_, nil)", err)
 	}
-	var icmpv6 testbench.ICMPv6 = testbench.ICMPv6{
-		Type: testbench.ICMPv6Type(header.ICMPv6DstUnreachable),
-		Code: testbench.ICMPv6Code(header.ICMPv6NetworkUnreachable),
-		// Per RFC 4443 3.1, the payload contains 4 zeroed bytes.
-		Payload: []byte{0, 0, 0, 0},
+
+	layers := conn.CreateFrame(t, nil)
+	layers[len(layers)-1] = &testbench.ICMPv6{
+		Type:    testbench.ICMPv6Type(header.ICMPv6DstUnreachable),
+		Code:    testbench.ICMPv6Code(header.ICMPv6NetworkUnreachable),
+		Payload: bytes,
 	}
-	layers = append(layers, &icmpv6, ip, tcp)
 	conn.SendFrameStateless(t, layers)
 
 	if err := getConnectError(t, &dut, clientFD); err != unix.ENETUNREACH {
diff --git a/test/packetimpact/tests/udp_icmp_error_propagation_test.go b/test/packetimpact/tests/udp_icmp_error_propagation_test.go
index 3159d5b89..087aeb66e 100644
--- a/test/packetimpact/tests/udp_icmp_error_propagation_test.go
+++ b/test/packetimpact/tests/udp_icmp_error_propagation_test.go
@@ -58,16 +58,20 @@ func (e icmpError) String() string {
 	return "Unknown ICMP error"
 }
 
-func (e icmpError) ToICMPv4() *testbench.ICMPv4 {
+func (e icmpError) ToICMPv4(payload []byte) *testbench.ICMPv4 {
 	switch e {
 	case portUnreachable:
 		return &testbench.ICMPv4{
-			Type: testbench.ICMPv4Type(header.ICMPv4DstUnreachable),
-			Code: testbench.ICMPv4Code(header.ICMPv4PortUnreachable)}
+			Type:    testbench.ICMPv4Type(header.ICMPv4DstUnreachable),
+			Code:    testbench.ICMPv4Code(header.ICMPv4PortUnreachable),
+			Payload: payload,
+		}
 	case timeToLiveExceeded:
 		return &testbench.ICMPv4{
-			Type: testbench.ICMPv4Type(header.ICMPv4TimeExceeded),
-			Code: testbench.ICMPv4Code(header.ICMPv4TTLExceeded)}
+			Type:    testbench.ICMPv4Type(header.ICMPv4TimeExceeded),
+			Code:    testbench.ICMPv4Code(header.ICMPv4TTLExceeded),
+			Payload: payload,
+		}
 	}
 	return nil
 }
@@ -101,8 +105,6 @@ func wantErrno(c connectionMode, icmpErr icmpError) unix.Errno {
 func sendICMPError(t *testing.T, conn *testbench.UDPIPv4, icmpErr icmpError, udp *testbench.UDP) {
 	t.Helper()
 
-	layers := conn.CreateFrame(t, nil)
-	layers = layers[:len(layers)-1]
 	ip, ok := udp.Prev().(*testbench.IPv4)
 	if !ok {
 		t.Fatalf("expected %s to be IPv4", udp.Prev())
@@ -113,12 +115,15 @@ func sendICMPError(t *testing.T, conn *testbench.UDPIPv4, icmpErr icmpError, udp
 		// to 1.
 		ip.Checksum = nil
 	}
-	// Note that the ICMP payload is valid in this case because the UDP
-	// payload is empty. If the UDP payload were not empty, the packet
-	// length during serialization may not be calculated correctly,
-	// resulting in a mal-formed packet.
-	layers = append(layers, icmpErr.ToICMPv4(), ip, udp)
 
+	icmpPayload := testbench.Layers{ip, udp}
+	bytes, err := icmpPayload.ToBytes()
+	if err != nil {
+		t.Fatalf("got icmpPayload.ToBytes() = (_, %s), want = (_, nil)", err)
+	}
+
+	layers := conn.CreateFrame(t, nil)
+	layers[len(layers)-1] = icmpErr.ToICMPv4(bytes)
 	conn.SendFrameStateless(t, layers)
 }