Teach netstack how to add options to IPv4 packets

Most packets don't have options but they are an integral part of the
standard. Teaching the ipv4 code how to handle them will simplify future
testing and use.  Because Options are so rare it is worth making sure
that the extra work is kept out of the fast path as much as possible.

Prior to this change, all usages of the IHL field of the IPv4Fields/Encode
system set it to the same constant value except in a couple of tests
for bad values. From this change IHL will not be a constant as it will
depend on the size of any Options. Since ipv4.Encode() now handles the
options it becomes a possible source of errors to let the callers set
this value, so remove it entirely and calculate the value from the size
of the Options if present (or not) therefore guaranteeing a correct value.

Fixes #4709
RELNOTES: n/a
PiperOrigin-RevId: 341864765

3 files changed, 47 insertions, 16 deletions

diff --git a/test/packetimpact/testbench/connections.go b/test/packetimpact/testbench/connections.go
index 8fa585804..030a73c3c 100644
--- a/test/packetimpact/testbench/connections.go
+++ b/test/packetimpact/testbench/connections.go
@@ -12,8 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package testbench has utilities to send and receive packets and also command
-// the DUT to run POSIX functions.
 package testbench
 
 import (
@@ -74,7 +72,7 @@ func pickPort(domain, typ int) (fd int, port uint16, err error) {
 	}
 	sa, err = unix.Getsockname(fd)
 	if err != nil {
-		return -1, 0, fmt.Errorf("Getsocketname(%d): %w", fd, err)
+		return -1, 0, fmt.Errorf("fail in Getsocketname(%d): %w", fd, err)
 	}
 	port, err = portFromSockaddr(sa)
 	if err != nil {
@@ -102,9 +100,9 @@ type layerState interface {
 	// as it was sent is available.
 	sent(sent Layer) error
 
-	// received updates the layerState based on a Layer that is receieved. The
+	// received updates the layerState based on a Layer that is received. The
 	// input is a Layer with all prev and next pointers populated so that the
-	// entire frame as it was receieved is available.
+	// entire frame as it was received is available.
 	received(received Layer) error
 
 	// close frees associated resources held by the LayerState.
@@ -475,12 +473,12 @@ func (conn *Connection) Close(t *testing.T) {
 	}
 }
 
-// CreateFrame builds a frame for the connection with defaults overriden
+// CreateFrame builds a frame for the connection with defaults overridden
 // from the innermost layer out, and additionalLayers added after it.
 //
 // Note that overrideLayers can have a length that is less than the number
 // of layers in this connection, and in such cases the innermost layers are
-// overriden first. As an example, valid values of overrideLayers for a TCP-
+// overridden first. As an example, valid values of overrideLayers for a TCP-
 // over-IPv4-over-Ethernet connection are: nil, [TCP], [IPv4, TCP], and
 // [Ethernet, IPv4, TCP].
 func (conn *Connection) CreateFrame(t *testing.T, overrideLayers Layers, additionalLayers ...Layer) Layers {
@@ -711,7 +709,7 @@ func (conn *TCPIPv4) ConnectWithOptions(t *testing.T, options []byte) {
 }
 
 // ExpectData is a convenient method that expects a Layer and the Layer after
-// it. If it doens't arrive in time, it returns nil.
+// it. If it doesn't arrive in time, it returns nil.
 func (conn *TCPIPv4) ExpectData(t *testing.T, tcp *TCP, payload *Payload, timeout time.Duration) (Layers, error) {
 	t.Helper()
 
@@ -1046,7 +1044,7 @@ func (conn *UDPIPv4) Expect(t *testing.T, udp UDP, timeout time.Duration) (*UDP,
 }
 
 // ExpectData is a convenient method that expects a Layer and the Layer after
-// it. If it doens't arrive in time, it returns nil.
+// it. If it doesn't arrive in time, it returns nil.
 func (conn *UDPIPv4) ExpectData(t *testing.T, udp UDP, payload Payload, timeout time.Duration) (Layers, error) {
 	t.Helper()
 
@@ -1174,7 +1172,7 @@ func (conn *UDPIPv6) Expect(t *testing.T, udp UDP, timeout time.Duration) (*UDP,
 }
 
 // ExpectData is a convenient method that expects a Layer and the Layer after
-// it. If it doens't arrive in time, it returns nil.
+// it. If it doesn't arrive in time, it returns nil.
 func (conn *UDPIPv6) ExpectData(t *testing.T, udp UDP, payload Payload, timeout time.Duration) (Layers, error) {
 	t.Helper()
 
@@ -1234,13 +1232,14 @@ func NewTCPIPv6(t *testing.T, outgoingTCP, incomingTCP TCP) TCPIPv6 {
 	}
 }
 
+// SrcPort returns the source port from the given Connection.
 func (conn *TCPIPv6) SrcPort() uint16 {
 	state := conn.layerStates[2].(*tcpState)
 	return *state.out.SrcPort
 }
 
 // ExpectData is a convenient method that expects a Layer and the Layer after
-// it. If it doens't arrive in time, it returns nil.
+// it. If it doesn't arrive in time, it returns nil.
 func (conn *TCPIPv6) ExpectData(t *testing.T, tcp *TCP, payload *Payload, timeout time.Duration) (Layers, error) {
 	t.Helper()
 
diff --git a/test/packetimpact/testbench/layers.go b/test/packetimpact/testbench/layers.go
index fc45d2085..2fb7ca9ba 100644
--- a/test/packetimpact/testbench/layers.go
+++ b/test/packetimpact/testbench/layers.go
@@ -286,6 +286,7 @@ type IPv4 struct {
 	Checksum       *uint16
 	SrcAddr        *tcpip.Address
 	DstAddr        *tcpip.Address
+	Options        *header.IPv4Options
 }
 
 func (l *IPv4) String() string {
@@ -294,10 +295,22 @@ func (l *IPv4) String() string {
 
 // ToBytes implements Layer.ToBytes.
 func (l *IPv4) ToBytes() ([]byte, error) {
-	b := make([]byte, header.IPv4MinimumSize)
+	// An IPv4 header is variable length depending on the size of the Options.
+	hdrLen := header.IPv4MinimumSize
+	if l.Options != nil {
+		hdrLen += l.Options.AllocationSize()
+		if hdrLen > header.IPv4MaximumHeaderSize {
+			// While ToBytes can be called on packets that were received as well
+			// as packets locally generated, it is physically impossible for a
+			// received packet to overflow this value so any such failure must
+			// be the result of a local programming error and not remotely
+			// triggered. A panic is therefore appropriate.
+			panic(fmt.Sprintf("IPv4 Options %d bytes, Max %d", len(*l.Options), header.IPv4MaximumOptionsSize))
+		}
+	}
+	b := make([]byte, hdrLen)
 	h := header.IPv4(b)
 	fields := &header.IPv4Fields{
-		IHL:            20,
 		TOS:            0,
 		TotalLength:    0,
 		ID:             0,
@@ -308,6 +321,11 @@ func (l *IPv4) ToBytes() ([]byte, error) {
 		Checksum:       0,
 		SrcAddr:        tcpip.Address(""),
 		DstAddr:        tcpip.Address(""),
+		Options:        nil,
+	}
+	// Leave an empty options slice as nil.
+	if hdrLen > header.IPv4MinimumSize {
+		fields.Options = *l.Options
 	}
 	if l.TOS != nil {
 		fields.TOS = *l.TOS
@@ -362,6 +380,11 @@ func (l *IPv4) ToBytes() ([]byte, error) {
 	if l.Checksum == nil {
 		h.SetChecksum(^h.CalculateChecksum())
 	}
+	// Encode cannot set this incorrectly so we need to overwrite what it wrote
+	// in order to test handling of a bad IHL value.
+	if l.IHL != nil {
+		h.SetHeaderLength(*l.IHL)
+	}
 	return h, nil
 }
 
@@ -377,8 +400,8 @@ func Uint8(v uint8) *uint8 {
 	return &v
 }
 
-// Address is a helper routine that allocates a new tcpip.Address value to store
-// v and returns a pointer to it.
+// Address is a helper routine that allocates a new tcpip.Address value to
+// store v and returns a pointer to it.
 func Address(v tcpip.Address) *tcpip.Address {
 	return &v
 }
@@ -387,6 +410,13 @@ func Address(v tcpip.Address) *tcpip.Address {
 // continues parsing further encapsulations.
 func parseIPv4(b []byte) (Layer, layerParser) {
 	h := header.IPv4(b)
+	hdrLen := h.HeaderLength()
+	// Even if there are no options, we set an empty options field instead of nil
+	// so that the decision to compare is up to the caller of that comparison.
+	var options header.IPv4Options
+	if hdrLen > header.IPv4MinimumSize {
+		options = append(options, h.Options()...)
+	}
 	tos, _ := h.TOS()
 	ipv4 := IPv4{
 		IHL:            Uint8(h.HeaderLength()),
@@ -400,6 +430,7 @@ func parseIPv4(b []byte) (Layer, layerParser) {
 		Checksum:       Uint16(h.Checksum()),
 		SrcAddr:        Address(h.SourceAddress()),
 		DstAddr:        Address(h.DestinationAddress()),
+		Options:        &options,
 	}
 	var nextParser layerParser
 	// If it is a fragment, don't treat it as having a transport protocol.
diff --git a/test/packetimpact/testbench/testbench.go b/test/packetimpact/testbench/testbench.go
index 3c85ebbee..c1db95d8c 100644
--- a/test/packetimpact/testbench/testbench.go
+++ b/test/packetimpact/testbench/testbench.go
@@ -12,7 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package testbench is the packetimpact test API.
+// Package testbench has utilities to send and receive packets, and also command
+// the DUT to run POSIX functions. It is the packetimpact test API.
 package testbench
 
 import (