mirror of
https://github.com/slackhq/nebula.git
synced 2026-08-15 11:27:02 +02:00
44dd2e9ca4
Multi-disciplinary correctness review of the batched tun / GSO-GRO / sendmmsg rework. Each fix has a regression test; the merged tree builds on linux/darwin/openbsd/windows/freebsd/netbsd, vets clean, passes the unit and e2e suites, and is -race clean. Critical: - C1 zero-length inner UDP datagram no longer panics the process (remote DoS): the UDP coalescer routes payLen==0 to passthrough instead of seeding a GSO slot, and WriteGSO skips empty payload iovecs as defense in depth. - C2 segmenter no longer corrupts inner headers when gsoSize < headerLen: the L3+L4 header is snapshotted once and each segment stamped from the copy, replacing the destructive overlapping in-place slide (SegmentTCP + SegmentUDP). High: - H1 applyOuterECN updates the IPv4 header checksum (RFC 1624 incremental) when folding outer CE into the inner ToS, so passthrough packets are no longer dropped by the peer stack. - H2 the GRO reject path caps the borrowed RX segment ([:n:n]) so a reject can no longer overrun into the next coalesced segment's Nebula header. Note: oversized ICMPv6 rejects that need >16B beyond the segment are now refused rather than sent under GRO (safe; see TOFIX.md for the scratch-buffer follow-up). - H3 WriteBatch falls back to per-packet WriteTo for a chunk when writeSockaddr fails, so one bad-family destination costs only its own packet, not the batch. - H4 UserDevice.Readers returns N distinct queue wrappers with private buffers (sharing the pipes) so concurrent readers no longer race/overwrite borrowed packet bytes. - H5 Poll.Close / Offload.Close no longer null t.fd (matching master's tunFile.Close), removing the data race with a concurrent readOne load. Medium/Low: - M1 the UDP GSO 127-segment gate moved from kernel >=5.5 to >=6.9 (the real UDP_MAX_SEGMENTS 64->128 threshold), avoiding EINVAL + per-packet fallback on 5.5-6.8 kernels. - M2 NewMultiQueueReader replays the offload mask newTun actually negotiated instead of the TSO-only mask, so adding a queue no longer disables USO device-wide; the advertised USO capability derives from the same mask. - M3 the shutdown eventfd is closed in pollQueueSet.Close / offloadQueueSet.Close (double-close guarded), fixing the per-lifecycle fd leak. - M4 dual-stack ECN selects the cmsg by address family, not socket family: RX parseRecvCmsg reads both IP_TOS and IPV6_TCLASS; TX writeEntryCmsg stamps IP_TOS for v4/v4-mapped dests and IPV6_TCLASS for v6 (on-host verified). - L1 newPoll no longer closes the fd on failure (matching newOffload), removing the double-close on QueueSet.Add error.
182 lines
5.0 KiB
Go
182 lines
5.0 KiB
Go
package overlay
|
|
|
|
import (
|
|
"fmt"
|
|
"net/netip"
|
|
"sync"
|
|
"testing"
|
|
|
|
"github.com/slackhq/nebula/overlay/tio"
|
|
)
|
|
|
|
// newTestUserDevice returns the concrete *UserDevice so tests can reach Pipe()
|
|
// and the internal queue plumbing.
|
|
func newTestUserDevice(t *testing.T) *UserDevice {
|
|
t.Helper()
|
|
dev, err := NewUserDevice([]netip.Prefix{netip.MustParsePrefix("10.0.0.1/24")})
|
|
if err != nil {
|
|
t.Fatalf("NewUserDevice: %v", err)
|
|
}
|
|
ud, ok := dev.(*UserDevice)
|
|
if !ok {
|
|
t.Fatalf("NewUserDevice returned %T, want *UserDevice", dev)
|
|
}
|
|
return ud
|
|
}
|
|
|
|
// TestUserDeviceReadersDistinctBuffers is the regression test for the
|
|
// multiqueue packet-corruption bug: Readers() used to hand the same
|
|
// *UserDevice (and therefore the same readBuf/batchRet) to every queue, so one
|
|
// reader's borrowed Packet.Bytes was overwritten by another reader's
|
|
// concurrent Read. Readers() must now return numReaders DISTINCT queue objects,
|
|
// each with its own backing buffer.
|
|
func TestUserDeviceReadersDistinctBuffers(t *testing.T) {
|
|
d := newTestUserDevice(t)
|
|
|
|
// One extra reader => two queues total.
|
|
if err := d.NewMultiQueueReader(); err != nil {
|
|
t.Fatalf("NewMultiQueueReader: %v", err)
|
|
}
|
|
|
|
readers := d.Readers()
|
|
if len(readers) != 2 {
|
|
t.Fatalf("Readers() returned %d queues, want 2", len(readers))
|
|
}
|
|
|
|
q0 := readers[0].(*userDeviceQueue)
|
|
q1 := readers[1].(*userDeviceQueue)
|
|
|
|
// Distinct queue objects.
|
|
if q0 == q1 {
|
|
t.Fatal("Readers() returned the same queue object twice")
|
|
}
|
|
// Distinct backing buffers (the actual regression: shared readBuf).
|
|
if &q0.readBuf[0] == &q1.readBuf[0] {
|
|
t.Fatal("queues share the same readBuf backing array")
|
|
}
|
|
// Shared underlying pipes.
|
|
if q0.outboundReader != q1.outboundReader || q0.inboundWriter != q1.inboundWriter {
|
|
t.Fatal("queues do not share the underlying pipes")
|
|
}
|
|
|
|
// Drive one packet through each queue and confirm the borrowed bytes from
|
|
// the first read are NOT clobbered by the second read. With a shared
|
|
// buffer, reading pkt1 into q1 would corrupt q0's still-borrowed slice.
|
|
_, ow := d.Pipe()
|
|
|
|
pkt0 := []byte("packet-zero-aaaaaaaa")
|
|
pkt1 := []byte("packet-one-bbbbbbbbb")
|
|
|
|
// The pipe is unbuffered, so writes block until a reader consumes them.
|
|
// Serialize: write pkt0 (read on q0), then write pkt1 (read on q1).
|
|
go func() {
|
|
if _, err := ow.Write(pkt0); err != nil {
|
|
t.Errorf("write pkt0: %v", err)
|
|
}
|
|
if _, err := ow.Write(pkt1); err != nil {
|
|
t.Errorf("write pkt1: %v", err)
|
|
}
|
|
}()
|
|
|
|
got0, err := readers[0].Read()
|
|
if err != nil {
|
|
t.Fatalf("q0.Read: %v", err)
|
|
}
|
|
if len(got0) != 1 || string(got0[0].Bytes) != string(pkt0) {
|
|
t.Fatalf("q0 first read = %q, want %q", firstBytes(got0), pkt0)
|
|
}
|
|
// Hold onto q0's borrowed slice across q1's read.
|
|
borrowed := got0[0].Bytes
|
|
|
|
got1, err := readers[1].Read()
|
|
if err != nil {
|
|
t.Fatalf("q1.Read: %v", err)
|
|
}
|
|
if len(got1) != 1 || string(got1[0].Bytes) != string(pkt1) {
|
|
t.Fatalf("q1 read = %q, want %q", firstBytes(got1), pkt1)
|
|
}
|
|
|
|
// q0's borrowed bytes must still hold pkt0 - a shared buffer would now
|
|
// show pkt1's contents.
|
|
if string(borrowed) != string(pkt0) {
|
|
t.Fatalf("q0 borrowed bytes were clobbered by q1's read: got %q, want %q", borrowed, pkt0)
|
|
}
|
|
}
|
|
|
|
// TestUserDeviceReadersConcurrentRace exercises two queues reading distinct
|
|
// packets concurrently. Run it under `go test -race`: with the old
|
|
// shared-buffer implementation the concurrent Reads raced on readBuf/batchRet
|
|
// and corrupted each other's returned slices.
|
|
func TestUserDeviceReadersConcurrentRace(t *testing.T) {
|
|
d := newTestUserDevice(t)
|
|
if err := d.NewMultiQueueReader(); err != nil {
|
|
t.Fatalf("NewMultiQueueReader: %v", err)
|
|
}
|
|
readers := d.Readers()
|
|
_, ow := d.Pipe()
|
|
|
|
const iterations = 200
|
|
|
|
errs := make(chan error, 3)
|
|
|
|
// Each reader parks in Read on the shared outboundReader; io.Pipe hands
|
|
// each write to whichever reader is currently waiting. We only care that
|
|
// concurrent Reads into distinct buffers are race-free, so any parked
|
|
// reader may serve any write.
|
|
var wg sync.WaitGroup
|
|
run := func(idx int) {
|
|
defer wg.Done()
|
|
for i := 0; i < iterations; i++ {
|
|
pkts, err := readers[idx].Read()
|
|
if err != nil {
|
|
errs <- err
|
|
return
|
|
}
|
|
if len(pkts) != 1 {
|
|
errs <- fmt.Errorf("reader %d: got %d packets, want 1", idx, len(pkts))
|
|
return
|
|
}
|
|
// Touch every byte of the borrowed slice while the other reader
|
|
// may be mid-Read; a shared buffer would race here.
|
|
total := 0
|
|
for _, c := range pkts[0].Bytes {
|
|
total += int(c)
|
|
}
|
|
_ = total
|
|
}
|
|
}
|
|
|
|
wg.Add(2)
|
|
go run(0)
|
|
go run(1)
|
|
|
|
// Feed 2*iterations packets. io.Pipe copies each write straight into the
|
|
// waiting reader's private buffer, so reusing buf between writes is safe.
|
|
go func() {
|
|
buf := make([]byte, 32)
|
|
for i := 0; i < 2*iterations; i++ {
|
|
for j := range buf {
|
|
buf[j] = byte(i + j)
|
|
}
|
|
if _, err := ow.Write(buf); err != nil {
|
|
errs <- err
|
|
return
|
|
}
|
|
}
|
|
}()
|
|
|
|
wg.Wait()
|
|
select {
|
|
case err := <-errs:
|
|
t.Fatalf("concurrent reader failed: %v", err)
|
|
default:
|
|
}
|
|
}
|
|
|
|
func firstBytes(p []tio.Packet) []byte {
|
|
if len(p) == 0 {
|
|
return nil
|
|
}
|
|
return p[0].Bytes
|
|
}
|