Requested Comment/Message wording changes

PiperOrigin-RevId: 342366891

pkg/tcpip/header/ipv4.go

@@ -89,7 +89,17 @@ type IPv4Fields struct {
 	// DstAddr is the "destination ip address" of an IPv4 packet.
 	DstAddr tcpip.Address
 
-	// Options is between 0 and 40 bytes or nil if empty.
+	// Options must be 40 bytes or less as they must fit along with the
+	// rest of the IPv4 header into the maximum size describable in the
+	// IHL field. RFC 791 section 3.1 says:
+	//    IHL:  4 bits
+	//
+	//    Internet Header Length is the length of the internet header in 32
+	//    bit words, and thus points to the beginning of the data.  Note that
+	//    the minimum value for a correct header is 5.
+	//
+	// That leaves ten 32 bit (4 byte) fields for options. An attempt to encode
+	// more will fail.
 	Options IPv4Options
 }
 
@@ -284,13 +294,10 @@ func (o IPv4Options) SizeWithPadding() int {
 	return (len(o) + IPv4IHLStride - 1) & ^(IPv4IHLStride - 1)
 }
 
-// Options returns a buffer holding the options or nil.
+// Options returns a buffer holding the options.
 func (b IPv4) Options() IPv4Options {
 	hdrLen := b.HeaderLength()
-	if hdrLen > IPv4MinimumSize {
-		return IPv4Options(b[options:hdrLen:hdrLen])
-	}
-	return nil
+	return IPv4Options(b[options:hdrLen:hdrLen])
 }
 
 // TransportProtocol implements Network.TransportProtocol.

pkg/tcpip/network/ipv4/icmp.go

@@ -90,7 +90,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) {
 
 	iph := header.IPv4(pkt.NetworkHeader().View())
 	var newOptions header.IPv4Options
-	if len(iph) > header.IPv4MinimumSize {
+	if opts := iph.Options(); len(opts) != 0 {
 		// RFC 1122 section 3.2.2.6 (page 43) (and similar for other round trip
 		// type ICMP packets):
 		//    If a Record Route and/or Time Stamp option is received in an
@@ -106,7 +106,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) {
 		} else {
 			op = &optionUsageReceive{}
 		}
-		aux, tmp, err := e.processIPOptions(pkt, iph.Options(), op)
+		aux, tmp, err := e.processIPOptions(pkt, opts, op)
 		if err != nil {
 			switch {
 			case

pkg/tcpip/network/ipv4/ipv4.go

@@ -713,11 +713,11 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) {
 		e.handleICMP(pkt)
 		return
 	}
-	if len(h.Options()) != 0 {
+	if opts := h.Options(); len(opts) != 0 {
 		// TODO(gvisor.dev/issue/4586):
 		// When we add forwarding support we should use the verified options
 		// rather than just throwing them away.
-		aux, _, err := e.processIPOptions(pkt, h.Options(), &optionUsageReceive{})
+		aux, _, err := e.processIPOptions(pkt, opts, &optionUsageReceive{})
 		if err != nil {
 			switch {
 			case

test/packetimpact/testbench/connections.go

@@ -72,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("fail in Getsocketname(%d): %w", fd, err)
+		return -1, 0, fmt.Errorf("unix.Getsocketname(%d): %w", fd, err)
 	}
 	port, err = portFromSockaddr(sa)
 	if err != nil {

test/packetimpact/testbench/layers.go

@@ -410,13 +410,7 @@ 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()...)
-	}
+	options := h.Options()
 	tos, _ := h.TOS()
 	ipv4 := IPv4{
 		IHL:            Uint8(h.HeaderLength()),