netstack: change PacketBufferPtr back to *PacketBuffer

The benefits of PacketBufferPtr never materialized and it makes the type
difficult to work with, e.g. it can't be used with go_generics to make a
(performant) list.

This is effectively a rollback of cl/480518221.

PiperOrigin-RevId: 530954630

nogo.yaml

@@ -253,3 +253,7 @@ analyzers:
       suppress:
         - "comment on exported type Translation" # Intentional.
         - "comment on exported type PinnedRange" # Intentional.
+  ST1016: # CheckReceiverNamesIdentical
+    internal:
+      exclude:
+        - pkg/tcpip/stack/packet_buffer.go # TODO(b/233086175): Remove.

pkg/tcpip/link/channel/channel.go

@@ -63,7 +63,7 @@ func (q *queue) Read() stack.PacketBufferPtr {
 	case p := <-q.c:
 		return p
 	default:
-		return stack.PacketBufferPtr{}
+		return nil
 	}
 }
 
@@ -72,7 +72,7 @@ func (q *queue) ReadContext(ctx context.Context) stack.PacketBufferPtr {
 	case pkt := <-q.c:
 		return pkt
 	case <-ctx.Done():
-		return stack.PacketBufferPtr{}
+		return nil
 	}
 }
 

pkg/tcpip/link/qdisc/fifo/packet_buffer_circular_list.go

@@ -71,10 +71,10 @@ func (pl *packetBufferCircularList) pushBack(pb stack.PacketBufferPtr) {
 //go:nosplit
 func (pl *packetBufferCircularList) removeFront() stack.PacketBufferPtr {
 	if pl.isEmpty() {
-		return stack.PacketBufferPtr{}
+		return nil
 	}
 	ret := pl.pbs[pl.head]
-	pl.pbs[pl.head] = stack.PacketBufferPtr{}
+	pl.pbs[pl.head] = nil
 	pl.head = (pl.head + 1) % len(pl.pbs)
 	pl.size--
 	return ret

pkg/tcpip/link/qdisc/fifo/qdisc_test.go

@@ -83,7 +83,7 @@ func TestWriteRefusedAfterClosed(t *testing.T) {
 	linkEp := fifo.New(nil, 1, 2)
 
 	linkEp.Close()
-	err := linkEp.WritePacket(stack.PacketBufferPtr{})
+	err := linkEp.WritePacket(nil)
 	_, ok := err.(*tcpip.ErrClosedForSend)
 	if !ok {
 		t.Errorf("got err = %s, want %s", err, &tcpip.ErrClosedForSend{})

pkg/tcpip/network/internal/fragmentation/fragmentation.go

@@ -158,25 +158,25 @@ func (f *Fragmentation) Process(
 	id FragmentID, first, last uint16, more bool, proto uint8, pkt stack.PacketBufferPtr) (
 	stack.PacketBufferPtr, uint8, bool, error) {
 	if first > last {
-		return stack.PacketBufferPtr{}, 0, false, fmt.Errorf("first=%d is greater than last=%d: %w", first, last, ErrInvalidArgs)
+		return nil, 0, false, fmt.Errorf("first=%d is greater than last=%d: %w", first, last, ErrInvalidArgs)
 	}
 
 	if first%f.blockSize != 0 {
-		return stack.PacketBufferPtr{}, 0, false, fmt.Errorf("first=%d is not a multiple of block size=%d: %w", first, f.blockSize, ErrInvalidArgs)
+		return nil, 0, false, fmt.Errorf("first=%d is not a multiple of block size=%d: %w", first, f.blockSize, ErrInvalidArgs)
 	}
 
 	fragmentSize := last - first + 1
 	if more && fragmentSize%f.blockSize != 0 {
-		return stack.PacketBufferPtr{}, 0, false, fmt.Errorf("fragment size=%d bytes is not a multiple of block size=%d on non-final fragment: %w", fragmentSize, f.blockSize, ErrInvalidArgs)
+		return nil, 0, false, fmt.Errorf("fragment size=%d bytes is not a multiple of block size=%d on non-final fragment: %w", fragmentSize, f.blockSize, ErrInvalidArgs)
 	}
 
 	if l := pkt.Data().Size(); l != int(fragmentSize) {
-		return stack.PacketBufferPtr{}, 0, false, fmt.Errorf("got fragment size=%d bytes not equal to the expected fragment size=%d bytes (first=%d last=%d): %w", l, fragmentSize, first, last, ErrInvalidArgs)
+		return nil, 0, false, fmt.Errorf("got fragment size=%d bytes not equal to the expected fragment size=%d bytes (first=%d last=%d): %w", l, fragmentSize, first, last, ErrInvalidArgs)
 	}
 
 	f.mu.Lock()
 	if f.reassemblers == nil {
-		return stack.PacketBufferPtr{}, 0, false, fmt.Errorf("Release() called before fragmentation processing could finish")
+		return nil, 0, false, fmt.Errorf("Release() called before fragmentation processing could finish")
 	}
 
 	r, ok := f.reassemblers[id]
@@ -201,7 +201,7 @@ func (f *Fragmentation) Process(
 		f.mu.Lock()
 		f.release(r, false /* timedOut */)
 		f.mu.Unlock()
-		return stack.PacketBufferPtr{}, 0, false, fmt.Errorf("fragmentation processing error: %w", err)
+		return nil, 0, false, fmt.Errorf("fragmentation processing error: %w", err)
 	}
 	f.mu.Lock()
 	f.memSize += memConsumed
@@ -253,12 +253,12 @@ func (f *Fragmentation) release(r *reassembler, timedOut bool) {
 	}
 	if !r.pkt.IsNil() {
 		r.pkt.DecRef()
-		r.pkt = stack.PacketBufferPtr{}
+		r.pkt = nil
 	}
 	for _, h := range r.holes {
 		if !h.pkt.IsNil() {
 			h.pkt.DecRef()
-			h.pkt = stack.PacketBufferPtr{}
+			h.pkt = nil
 		}
 	}
 	r.holes = nil

pkg/tcpip/network/internal/fragmentation/fragmentation_test.go

@@ -632,7 +632,7 @@ func TestTimeoutHandler(t *testing.T) {
 				},
 			},
 			wantError: false,
-			wantPkt:   stack.PacketBufferPtr{},
+			wantPkt:   nil,
 		},
 		{
 			name: "second pkt is ignored",
@@ -664,7 +664,7 @@ func TestTimeoutHandler(t *testing.T) {
 				},
 			},
 			wantError: true,
-			wantPkt:   stack.PacketBufferPtr{},
+			wantPkt:   nil,
 		},
 	}
 
@@ -672,7 +672,7 @@ func TestTimeoutHandler(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			handler := &testTimeoutHandler{pkt: stack.PacketBufferPtr{}}
+			handler := &testTimeoutHandler{pkt: nil}
 
 			f := NewFragmentation(minBlockSize, HighFragThreshold, LowFragThreshold, reassembleTimeout, &faketime.NullClock{}, handler)
 
@@ -703,7 +703,7 @@ func TestTimeoutHandler(t *testing.T) {
 }
 
 func TestFragmentSurvivesReleaseJob(t *testing.T) {
-	handler := &testTimeoutHandler{pkt: stack.PacketBufferPtr{}}
+	handler := &testTimeoutHandler{pkt: nil}
 	c := faketime.NewManualClock()
 	f := NewFragmentation(minBlockSize, HighFragThreshold, LowFragThreshold, reassembleTimeout, c, handler)
 	pkt := pkt(2, "01")

pkg/tcpip/network/internal/fragmentation/reassembler.go

@@ -67,7 +67,7 @@ func (r *reassembler) process(first, last uint16, more bool, proto uint8, pkt st
 		// A concurrent goroutine might have already reassembled
 		// the packet and emptied the heap while this goroutine
 		// was waiting on the mutex. We don't have to do anything in this case.
-		return stack.PacketBufferPtr{}, 0, false, 0, nil
+		return nil, 0, false, 0, nil
 	}
 
 	var holeFound bool
@@ -91,12 +91,12 @@ func (r *reassembler) process(first, last uint16, more bool, proto uint8, pkt st
 		// https://github.com/torvalds/linux/blob/38525c6/net/ipv4/inet_fragment.c#L349
 		if first < currentHole.first || currentHole.last < last {
 			// Incoming fragment only partially fits in the free hole.
-			return stack.PacketBufferPtr{}, 0, false, 0, ErrFragmentOverlap
+			return nil, 0, false, 0, ErrFragmentOverlap
 		}
 		if !more {
 			if !currentHole.final || currentHole.filled && currentHole.last != last {
 				// We have another final fragment, which does not perfectly overlap.
-				return stack.PacketBufferPtr{}, 0, false, 0, ErrFragmentConflict
+				return nil, 0, false, 0, ErrFragmentConflict
 			}
 		}
 
@@ -155,12 +155,12 @@ func (r *reassembler) process(first, last uint16, more bool, proto uint8, pkt st
 	}
 	if !holeFound {
 		// Incoming fragment is beyond end.
-		return stack.PacketBufferPtr{}, 0, false, 0, ErrFragmentConflict
+		return nil, 0, false, 0, ErrFragmentConflict
 	}
 
 	// Check if all the holes have been filled and we are ready to reassemble.
 	if r.filled < len(r.holes) {
-		return stack.PacketBufferPtr{}, 0, false, memConsumed, nil
+		return nil, 0, false, memConsumed, nil
 	}
 
 	sort.Slice(r.holes, func(i, j int) bool {

pkg/tcpip/network/ipv4/icmp.go

@@ -361,8 +361,7 @@ func (e *endpoint) handleICMP(pkt stack.PacketBufferPtr) {
 
 		// It's possible that a raw socket expects to receive this.
 		e.dispatcher.DeliverTransportPacket(header.ICMPv4ProtocolNumber, pkt)
-		pkt = stack.PacketBufferPtr{}
-		_ = pkt // Suppress unused variable warning.
+		pkt = nil
 
 		sent := e.stats.icmp.packetsSent
 		if !e.protocol.allowICMPReply(header.ICMPv4EchoReply, header.ICMPv4UnusedCode) {

pkg/tcpip/stack/BUILD

@@ -179,7 +179,7 @@ go_template_instance(
     prefix = "packetBuffer",
     template = "//pkg/refs:refs_template",
     types = {
-        "T": "packetBuffer",
+        "T": "PacketBuffer",
     },
 )
 

pkg/tcpip/stack/gro.go

@@ -249,7 +249,7 @@ func (gb *groBucket) found(gd *groDispatcher, groPkt *groPacket, flushGROPkt boo
 		// Add flags from the packet to the GRO packet.
 		groPkt.tcpHdr.SetFlags(uint8(groPkt.tcpHdr.Flags() | (flags & (header.TCPFlagFin | header.TCPFlagPsh))))
 
-		pkt = PacketBufferPtr{}
+		pkt = nil
 	}
 
 	// Flush if the packet isn't the same size as the previous packets or

pkg/tcpip/stack/packet_buffer.go

@@ -35,7 +35,7 @@ const (
 
 var pkPool = sync.Pool{
 	New: func() any {
-		return &packetBuffer{}
+		return &PacketBuffer{}
 	},
 }
 
@@ -59,14 +59,9 @@ type PacketBufferOptions struct {
 }
 
 // PacketBufferPtr is a pointer to a PacketBuffer.
-//
-// +stateify savable
-type PacketBufferPtr struct {
-	// packetBuffer is the underlying packet buffer.
-	*packetBuffer
-}
+type PacketBufferPtr = *PacketBuffer
 
-// A packetBuffer contains all the data of a network packet.
+// A PacketBuffer contains all the data of a network packet.
 //
 // As a PacketBuffer traverses up the stack, it may be necessary to pass it to
 // multiple endpoints.
@@ -108,7 +103,7 @@ type PacketBufferPtr struct {
 // starting offset of each header in `buf`.
 //
 // +stateify savable
-type packetBuffer struct {
+type PacketBuffer struct {
 	_ sync.NoCopy
 
 	packetBufferRefs
@@ -178,7 +173,7 @@ type packetBuffer struct {
 
 // NewPacketBuffer creates a new PacketBuffer with opts.
 func NewPacketBuffer(opts PacketBufferOptions) PacketBufferPtr {
-	pk := pkPool.Get().(*packetBuffer)
+	pk := pkPool.Get().(*PacketBuffer)
 	pk.reset()
 	if opts.ReserveHeaderBytes != 0 {
 		v := bufferv2.NewViewSize(opts.ReserveHeaderBytes)
@@ -191,36 +186,31 @@ func NewPacketBuffer(opts PacketBufferOptions) PacketBufferPtr {
 	pk.NetworkPacketInfo.IsForwardedPacket = opts.IsForwardedPacket
 	pk.onRelease = opts.OnRelease
 	pk.InitRefs()
-	return PacketBufferPtr{
-		packetBuffer: pk,
-	}
+	return pk
 }
 
 // IncRef increments the PacketBuffer's refcount.
 func (pk PacketBufferPtr) IncRef() PacketBufferPtr {
 	pk.packetBufferRefs.IncRef()
-	return PacketBufferPtr{
-		packetBuffer: pk.packetBuffer,
-	}
+	return pk
 }
 
 // DecRef decrements the PacketBuffer's refcount. If the refcount is
 // decremented to zero, the PacketBuffer is returned to the PacketBuffer
 // pool.
-func (pk *PacketBufferPtr) DecRef() {
+func (pk PacketBufferPtr) DecRef() {
 	pk.packetBufferRefs.DecRef(func() {
 		if pk.onRelease != nil {
 			pk.onRelease()
 		}
 
 		pk.buf.Release()
-		pkPool.Put(pk.packetBuffer)
+		pkPool.Put(pk)
 	})
-	pk.packetBuffer = nil
 }
 
-func (pk *packetBuffer) reset() {
-	*pk = packetBuffer{}
+func (pk PacketBufferPtr) reset() {
+	*pk = PacketBuffer{}
 }
 
 // ReservedHeaderBytes returns the number of bytes initially reserved for
@@ -374,7 +364,7 @@ func (pk PacketBufferPtr) headerView(typ headerType) bufferv2.View {
 // Clone makes a semi-deep copy of pk. The underlying packet payload is
 // shared. Hence, no modifications is done to underlying packet payload.
 func (pk PacketBufferPtr) Clone() PacketBufferPtr {
-	newPk := pkPool.Get().(*packetBuffer)
+	newPk := pkPool.Get().(*PacketBuffer)
 	newPk.reset()
 	newPk.buf = pk.buf.Clone()
 	newPk.reserved = pk.reserved
@@ -394,9 +384,7 @@ func (pk PacketBufferPtr) Clone() PacketBufferPtr {
 	newPk.NetworkPacketInfo = pk.NetworkPacketInfo
 	newPk.tuple = pk.tuple
 	newPk.InitRefs()
-	return PacketBufferPtr{
-		packetBuffer: newPk,
-	}
+	return newPk
 }
 
 // ReserveHeaderBytes prepends reserved space for headers at the front
@@ -429,16 +417,14 @@ func (pk PacketBufferPtr) Network() header.Network {
 // See PacketBuffer.Data for details about how a packet buffer holds an inbound
 // packet.
 func (pk PacketBufferPtr) CloneToInbound() PacketBufferPtr {
-	newPk := pkPool.Get().(*packetBuffer)
+	newPk := pkPool.Get().(*PacketBuffer)
 	newPk.reset()
 	newPk.buf = pk.buf.Clone()
 	newPk.InitRefs()
 	// Treat unfilled header portion as reserved.
 	newPk.reserved = pk.AvailableHeaderBytes()
 	newPk.tuple = pk.tuple
-	return PacketBufferPtr{
-		packetBuffer: newPk,
-	}
+	return newPk
 }
 
 // DeepCopyForForwarding creates a deep copy of the packet buffer for
@@ -476,7 +462,7 @@ func (pk PacketBufferPtr) DeepCopyForForwarding(reservedHeaderBytes int) PacketB
 
 // IsNil returns whether the pointer is logically nil.
 func (pk PacketBufferPtr) IsNil() bool {
-	return pk.packetBuffer == nil
+	return pk == nil
 }
 
 // headerInfo stores metadata about a header in a packet.

pkg/tcpip/stack/packet_buffer_list.go

@@ -37,7 +37,7 @@ func (pl *PacketBufferList) AsSlice() []PacketBufferPtr {
 func (pl *PacketBufferList) Reset() {
 	for i, pb := range pl.pbs {
 		pb.DecRef()
-		pl.pbs[i] = PacketBufferPtr{}
+		pl.pbs[i] = nil
 	}
 	pl.pbs = pl.pbs[:0]
 }

pkg/tcpip/stack/packet_buffer_unsafe.go

@@ -17,12 +17,12 @@ package stack
 import "unsafe"
 
 // PacketBufferStructSize is the minimal size of the packet buffer overhead.
-const PacketBufferStructSize = int(unsafe.Sizeof(packetBuffer{}))
+const PacketBufferStructSize = int(unsafe.Sizeof(PacketBuffer{}))
 
 // ID returns a unique ID for the underlying storage of the packet.
 //
 // Two PacketBufferPtrs have the same IDs if and only if they point to the same
 // location in memory.
 func (pk PacketBufferPtr) ID() uintptr {
-	return uintptr(unsafe.Pointer(pk.packetBuffer))
+	return uintptr(unsafe.Pointer(pk))
 }

pkg/tcpip/transport/internal/network/endpoint.go

@@ -272,7 +272,7 @@ func (c *WriteContext) TryNewPacketBuffer(reserveHdrBytes int, data bufferv2.Buf
 	defer e.sendBufferSizeInUseMu.Unlock()
 
 	if !e.hasSendSpaceRLocked() {
-		return stack.PacketBufferPtr{}
+		return nil
 	}
 
 	// Note that we allow oversubscription - if there is any space at all in the

pkg/tcpip/transport/tcp/segment.go

@@ -194,6 +194,7 @@ func (s *segment) DecRef() {
 			}
 		}
 		s.pkt.DecRef()
+		s.pkt = nil
 		segmentPool.Put(s)
 	})
 }