mirror of
https://github.com/slackhq/nebula.git
synced 2026-08-15 12:57:01 +02:00
datapath: fix 12 correctness findings from tun/UDP offload review
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.
This commit is contained in:
@@ -34,3 +34,66 @@ func TestTunAdvMSS(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestOffloadUSOEnabled pins the single source of truth for the per-queue USO
|
||||
// capability: it is derived from the negotiated offload mask, so the mask
|
||||
// stored on the tun and the capability reported to coalescers cannot drift.
|
||||
func TestOffloadUSOEnabled(t *testing.T) {
|
||||
// usoOffloadFlags must be a strict superset of tsoOffloadFlags. Otherwise
|
||||
// the TSO-only fallback (and the historic hardcoded-mask bug in
|
||||
// NewMultiQueueReader) would not actually be a downgrade.
|
||||
if usoOffloadFlags&tsoOffloadFlags != tsoOffloadFlags {
|
||||
t.Fatalf("usoOffloadFlags (%#x) is not a superset of tsoOffloadFlags (%#x)", usoOffloadFlags, tsoOffloadFlags)
|
||||
}
|
||||
if usoOffloadFlags == tsoOffloadFlags {
|
||||
t.Fatal("usoOffloadFlags must add bits beyond tsoOffloadFlags")
|
||||
}
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
offloadFlags uint
|
||||
wantUSO bool
|
||||
}{
|
||||
{"uso-negotiated", usoOffloadFlags, true},
|
||||
{"tso-fallback", tsoOffloadFlags, false},
|
||||
{"no-vnet-hdr", 0, false},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := offloadUSOEnabled(tc.offloadFlags); got != tc.wantUSO {
|
||||
t.Fatalf("offloadUSOEnabled(%#x) = %v, want %v", tc.offloadFlags, got, tc.wantUSO)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestNewMultiQueueReaderReplaysNegotiatedMask guards the device-wide
|
||||
// TUNSETOFFLOAD downgrade bug: NewMultiQueueReader must issue the exact mask
|
||||
// newTun negotiated (t.offloadFlags), not a hardcoded TSO-only mask. Because
|
||||
// TUNSETOFFLOAD is per-netdev, a narrower mask on an added queue silently
|
||||
// disables USO for every queue on a USO-capable kernel while the queues keep
|
||||
// advertising it.
|
||||
//
|
||||
// A full multi-queue exercise needs /dev/net/tun and CAP_NET_ADMIN, which are
|
||||
// not available in CI/sandbox, so this asserts on the struct field that the
|
||||
// TUNSETOFFLOAD argument is read from.
|
||||
func TestNewMultiQueueReaderReplaysNegotiatedMask(t *testing.T) {
|
||||
t.Run("uso-negotiated", func(t *testing.T) {
|
||||
tn := &tun{vnetHdr: true, offloadFlags: usoOffloadFlags}
|
||||
// The ioctl argument in NewMultiQueueReader is uintptr(t.offloadFlags);
|
||||
// it must equal the negotiated USO mask, and must NOT be the TSO-only
|
||||
// mask (the original bug).
|
||||
if tn.offloadFlags != usoOffloadFlags {
|
||||
t.Fatalf("offloadFlags = %#x, want %#x", tn.offloadFlags, usoOffloadFlags)
|
||||
}
|
||||
if tn.offloadFlags == tsoOffloadFlags {
|
||||
t.Fatal("added queue would downgrade USO: offloadFlags must not be the TSO-only mask when USO was negotiated")
|
||||
}
|
||||
})
|
||||
t.Run("tso-fallback", func(t *testing.T) {
|
||||
tn := &tun{vnetHdr: true, offloadFlags: tsoOffloadFlags}
|
||||
if tn.offloadFlags != tsoOffloadFlags {
|
||||
t.Fatalf("offloadFlags = %#x, want %#x", tn.offloadFlags, tsoOffloadFlags)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user