more fixes!

This commit is contained in:
JackDoan
2026-07-13 17:54:17 -05:00
parent c7918d1096
commit 1aca2f75ae
20 changed files with 489 additions and 241 deletions
+9 -18
View File
@@ -93,8 +93,11 @@ func parseIPPrologue(pkt []byte, wantProto byte) (parsedIP, bool) {
// ipHeadersMatch compares the IP portion of two packet header prefixes for
// byte-for-byte equality on every field that must be identical across
// coalesced segments. Size/IPID/IPCsum and the 2-bit IP-level ECN field are
// masked out — the appendPayload step merges CE into the seed.
// coalesced segments. Size/IPID/IPCsum are masked out. The full DSCP/ECN
// byte (IPv4 ToS / IPv6 traffic class) is compared, matching Linux kernel
// GRO: segments with differing ECN codepoints must not coalesce, otherwise
// ORing e.g. ECT(0) with ECT(1) would fabricate a false CE (congestion)
// mark or mark a Not-ECT flow as ECN-capable.
//
// The transport (L4) portion of the header is checked separately by the
// per-protocol matcher.
@@ -102,11 +105,11 @@ func ipHeadersMatch(a, b []byte, isV6 bool) bool {
if isV6 {
// IPv6: byte 0 = version/TC[7:4], byte 1 = TC[3:0]/flow[19:16],
// bytes [2:4] = flow[15:0], [6:8] = next_hdr/hop, [8:40] = src+dst.
// ECN lives in TC[1:0] = byte 1 mask 0x30. Skip [4:6] payload_len.
// Compare byte 1 fully so ECN (TC[1:0]) must match. Skip [4:6] payload_len.
if a[0] != b[0] {
return false
}
if a[1]&^0x30 != b[1]&^0x30 {
if a[1] != b[1] {
return false
}
if !bytes.Equal(a[2:4], b[2:4]) {
@@ -119,11 +122,12 @@ func ipHeadersMatch(a, b []byte, isV6 bool) bool {
}
// IPv4: byte 0 = version/IHL, byte 1 = DSCP(6)|ECN(2),
// [6:10] flags/fragoff/TTL/proto, [12:20] src+dst.
// Compare byte 1 fully so ECN must match.
// Skip [2:4] total len, [4:6] id, [10:12] csum.
if a[0] != b[0] {
return false
}
if a[1]&^0x03 != b[1]&^0x03 {
if a[1] != b[1] {
return false
}
if !bytes.Equal(a[6:10], b[6:10]) {
@@ -135,19 +139,6 @@ func ipHeadersMatch(a, b []byte, isV6 bool) bool {
return true
}
// mergeECNIntoSeed ORs the 2-bit IP-level ECN field of pkt's IP header
// onto the seed's IP header, so a CE mark on any coalesced segment
// propagates to the final superpacket. (CE is 0b11; ORing yields CE if
// any segment carried it.) Used by both TCP and UDP coalescers, so the
// invariant lives in one place.
func mergeECNIntoSeed(seedHdr, pktHdr []byte, isV6 bool) {
if isV6 {
seedHdr[1] |= pktHdr[1] & 0x30
} else {
seedHdr[1] |= pktHdr[1] & 0x03
}
}
// Arena is an injectable byte-slab that hands out non-overlapping borrowed
// slices via Reserve and releases them in bulk via Reset. Coalescers take
// an *Arena at construction so the caller controls the slab lifetime and
+9 -10
View File
@@ -365,9 +365,6 @@ func (c *TCPCoalescer) appendPayload(s *coalesceSlot, pkt []byte, info parsedTCP
// last segment. Without this the sender's push signal is dropped.
s.hdrBuf[s.ipHdrLen+13] |= tcpFlagPsh
}
// Merge IP-level CE marks into the seed: headersMatch ignores ECN, so
// this is the one place the signal is preserved.
mergeECNIntoSeed(s.hdrBuf[:s.ipHdrLen], pkt[:s.ipHdrLen], s.isV6)
if info.payLen < s.gsoSize || info.flags&tcpFlagPsh != 0 {
s.psh = true
}
@@ -424,8 +421,9 @@ func (c *TCPCoalescer) flushSlot(s *coalesceSlot) error {
// headersMatch compares two IP+TCP header prefixes for byte-for-byte
// equality on every field that must be identical across coalesced
// segments. Size/IPID/IPCsum/seq/flags/tcpCsum are masked out, as is the
// 2-bit IP-level ECN field — appendPayload merges CE into the seed.
// segments. Size/IPID/IPCsum/seq/flags/tcpCsum are masked out. The IP-level
// ECN codepoint is compared (via ipHeadersMatch) so segments with differing
// ECN don't coalesce, matching kernel GRO.
func headersMatch(a, b []byte, isV6 bool, ipHdrLen int) bool {
if len(a) != len(b) {
return false
@@ -632,6 +630,9 @@ func flowKeyCompare(a, b flowKey) int {
// ECE state must agree across both slots: PSH is a semantic delimiter
// (preserving the sender's push boundary) and ECE state must be uniform
// across a window (the same rule canAppend enforces for in-flow appends).
// The IP-level ECN codepoint must also match: this check calls headersMatch
// → ipHeadersMatch, which compares the full DSCP/ECN byte, so two slots with
// differing ECN marks stay separate superpackets, each keeping its own mark.
//
// Note: a slot sealed by reorder (canAppend returned false on seq
// mismatch) keeps psh=false, so this restriction does not block the
@@ -670,10 +671,9 @@ func canMergeSlots(prev, s *coalesceSlot) bool {
}
// mergeSlots folds src into dst in place: payIovs concatenated, counters
// and totals updated, PSH and IP-level CE bits OR'd into the seed header
// so neither the push signal nor a CE mark is lost. The seed header's
// seq, gsoSize, and fk are unchanged. Caller is responsible for releasing
// src (it's no longer in c.slots after this call).
// and totals updated, PSH OR'd into the seed header so the push signal is
// not lost. The seed header's seq, gsoSize, and fk are unchanged. Caller
// is responsible for releasing src (it's no longer in c.slots after this call).
func mergeSlots(dst, src *coalesceSlot) {
dst.payIovs = append(dst.payIovs, src.payIovs...)
dst.numSeg += src.numSeg
@@ -683,7 +683,6 @@ func mergeSlots(dst, src *coalesceSlot) {
dst.psh = true
dst.hdrBuf[dst.ipHdrLen+13] |= tcpFlagPsh
}
mergeECNIntoSeed(dst.hdrBuf[:dst.ipHdrLen], src.hdrBuf[:src.ipHdrLen], dst.isV6)
}
// ipv4HdrChecksum computes the IPv4 header checksum over hdr (which must
+90 -24
View File
@@ -762,39 +762,88 @@ func TestCoalescerEceMismatchReseeds(t *testing.T) {
}
}
// TestCoalescerMergesCEMark confirms that an ECT(0) burst with a single
// CE-marked packet still coalesces, and the merged superpacket carries CE.
func TestCoalescerMergesCEMark(t *testing.T) {
// TestCoalescerDifferingECNReseeds confirms that segments with differing IP
// ECN codepoints do NOT coalesce: headersMatch compares the full ToS byte,
// matching kernel GRO. Two ECT(0) segments merge; a CE stamp mid-run seals
// the ECT(0) chain and starts a fresh superpacket that keeps CE; a trailing
// ECT(0) starts yet another. Each superpacket keeps its own codepoint —
// ORing the marks (the old buggy behavior) would have fabricated a false CE
// across the whole burst.
func TestCoalescerDifferingECNReseeds(t *testing.T) {
w := &fakeTunWriter{gsoEnabled: true}
c := NewTCPCoalescer(w, test.NewLogger(), NewArena(0))
pay := make([]byte, 1200)
if err := c.Commit(buildTCPv4WithToS(ecnECT0, 1000, tcpAck, pay)); err != nil {
t.Fatal(err)
}
// Router along the path stamped CE on this one.
if err := c.Commit(buildTCPv4WithToS(ecnCE, 2200, tcpAck, pay)); err != nil {
if err := c.Commit(buildTCPv4WithToS(ecnECT0, 2200, tcpAck, pay)); err != nil {
t.Fatal(err)
}
if err := c.Commit(buildTCPv4WithToS(ecnECT0, 3400, tcpAck, pay)); err != nil {
// Router along the path stamped CE on this one.
if err := c.Commit(buildTCPv4WithToS(ecnCE, 3400, tcpAck, pay)); err != nil {
t.Fatal(err)
}
if err := c.Commit(buildTCPv4WithToS(ecnECT0, 4600, tcpAck, pay)); err != nil {
t.Fatal(err)
}
if err := c.Flush(); err != nil {
t.Fatal(err)
}
if len(w.gsoWrites) != 1 {
t.Fatalf("want 1 merged gso write, got %d (plain=%d)", len(w.gsoWrites), len(w.writes))
if len(w.gsoWrites) != 3 {
t.Fatalf("want 3 superpackets (ECN split), got %d (plain=%d)", len(w.gsoWrites), len(w.writes))
}
g := w.gsoWrites[0]
if len(g.pays) != 3 {
t.Errorf("pay count=%d want 3", len(g.pays))
// gso[0]: the two ECT(0) segments merged; gso[1]: CE alone; gso[2]:
// trailing ECT(0) alone. Emitted in seq order.
type want struct {
pays int
ecn byte
}
if got := g.hdr[1] & 0x03; got != ecnCE {
t.Errorf("seed ECN=0x%02x want CE 0x%02x", got, ecnCE)
wants := []want{{2, ecnECT0}, {1, ecnCE}, {1, ecnECT0}}
for i, wnt := range wants {
g := w.gsoWrites[i]
if len(g.pays) != wnt.pays {
t.Errorf("gso %d pay count=%d want %d", i, len(g.pays), wnt.pays)
}
if got := g.hdr[1] & 0x03; got != wnt.ecn {
t.Errorf("gso %d ECN=0x%02x want 0x%02x", i, got, wnt.ecn)
}
}
}
// TestCoalescerDscpMismatchReseeds confirms that the new ECN-mask in
// headersMatch did not also relax DSCP — different DSCP must still split.
// TestCoalescerECT0ThenECT1NoCE is the core regression for the ECN merge
// bug: ORing ECT(0)=0b10 with ECT(1)=0b01 fabricates CE=0b11. The two
// segments must land in separate superpackets, each preserving its own
// codepoint, and neither may end up CE-marked.
func TestCoalescerECT0ThenECT1NoCE(t *testing.T) {
w := &fakeTunWriter{gsoEnabled: true}
c := NewTCPCoalescer(w, test.NewLogger(), NewArena(0))
pay := make([]byte, 1200)
if err := c.Commit(buildTCPv4WithToS(ecnECT0, 1000, tcpAck, pay)); err != nil {
t.Fatal(err)
}
if err := c.Commit(buildTCPv4WithToS(ecnECT1, 2200, tcpAck, pay)); err != nil {
t.Fatal(err)
}
if err := c.Flush(); err != nil {
t.Fatal(err)
}
if len(w.gsoWrites) != 2 {
t.Fatalf("want 2 separate superpackets (ECT0 vs ECT1), got %d", len(w.gsoWrites))
}
wantECN := []byte{ecnECT0, ecnECT1}
for i, g := range w.gsoWrites {
if got := g.hdr[1] & 0x03; got != wantECN[i] {
t.Errorf("gso %d ECN=0x%02x want 0x%02x", i, got, wantECN[i])
}
if got := g.hdr[1] & 0x03; got == ecnCE {
t.Errorf("gso %d fabricated CE from ECT merge", i)
}
}
}
// TestCoalescerDscpMismatchReseeds confirms that a DSCP difference (same
// ECN) still splits — headersMatch compares the full ToS byte, so the upper
// six DSCP bits must match too.
func TestCoalescerDscpMismatchReseeds(t *testing.T) {
w := &fakeTunWriter{gsoEnabled: true}
c := NewTCPCoalescer(w, test.NewLogger(), NewArena(0))
@@ -995,9 +1044,10 @@ func TestCoalescerSortKeepsPassthroughBarrier(t *testing.T) {
}
}
// TestCoalescerIPv6MergesCEMark is the IPv6 analogue of
// TestCoalescerMergesCEMark. ECN bits live in TC[1:0] = byte 1 mask 0x30.
func TestCoalescerIPv6MergesCEMark(t *testing.T) {
// TestCoalescerIPv6DifferingECNReseeds is the IPv6 analogue of
// TestCoalescerDifferingECNReseeds. ECN bits live in TC[1:0] = byte 1 mask
// 0x30, so ipHeadersMatch (comparing byte 1 fully) still splits them.
func TestCoalescerIPv6DifferingECNReseeds(t *testing.T) {
w := &fakeTunWriter{gsoEnabled: true}
c := NewTCPCoalescer(w, test.NewLogger(), NewArena(0))
pay := make([]byte, 1200)
@@ -1005,20 +1055,36 @@ func TestCoalescerIPv6MergesCEMark(t *testing.T) {
if err := c.Commit(buildTCPv6(ecnECT0, 1000, tcpAck, pay)); err != nil {
t.Fatal(err)
}
if err := c.Commit(buildTCPv6(ecnCE, 2200, tcpAck, pay)); err != nil {
if err := c.Commit(buildTCPv6(ecnECT0, 2200, tcpAck, pay)); err != nil {
t.Fatal(err)
}
if err := c.Commit(buildTCPv6(ecnCE, 3400, tcpAck, pay)); err != nil {
t.Fatal(err)
}
if err := c.Commit(buildTCPv6(ecnECT0, 4600, tcpAck, pay)); err != nil {
t.Fatal(err)
}
if err := c.Flush(); err != nil {
t.Fatal(err)
}
if len(w.gsoWrites) != 1 {
t.Fatalf("want 1 merged gso write, got %d", len(w.gsoWrites))
if len(w.gsoWrites) != 3 {
t.Fatalf("want 3 superpackets (ECN split), got %d", len(w.gsoWrites))
}
g := w.gsoWrites[0]
// Byte 1 high nibble holds TC[3:0]; ECN is the low 2 bits of that nibble,
// which appears in byte 1 mask 0x30 (>>4 to read the codepoint value).
if got := (g.hdr[1] >> 4) & 0x03; got != ecnCE {
t.Errorf("seed v6 ECN=0x%02x want CE 0x%02x", got, ecnCE)
type want struct {
pays int
ecn byte
}
wants := []want{{2, ecnECT0}, {1, ecnCE}, {1, ecnECT0}}
for i, wnt := range wants {
g := w.gsoWrites[i]
if len(g.pays) != wnt.pays {
t.Errorf("gso %d pay count=%d want %d", i, len(g.pays), wnt.pays)
}
if got := (g.hdr[1] >> 4) & 0x03; got != wnt.ecn {
t.Errorf("gso %d v6 ECN=0x%02x want 0x%02x", i, got, wnt.ecn)
}
}
}
+3 -4
View File
@@ -257,8 +257,6 @@ func (c *UDPCoalescer) appendPayload(s *udpSlot, pkt []byte, info parsedUDP) {
s.payIovs = append(s.payIovs, pkt[info.hdrLen:info.hdrLen+info.payLen])
s.numSeg++
s.totalPay += info.payLen
// Merge IP-level CE marks into the seed (same trick TCP coalescer uses).
mergeECNIntoSeed(s.hdrBuf[:s.ipHdrLen], pkt[:s.ipHdrLen], s.isV6)
if info.payLen < s.gsoSize {
// Last-segment-can-be-shorter: this seals the chain.
s.sealed = true
@@ -329,8 +327,9 @@ func (c *UDPCoalescer) flushSlot(s *udpSlot) error {
// udpHeadersMatch compares two IP+UDP header prefixes for byte-equality on
// every field that must be identical across coalesced segments. Length
// fields and the ECN bits in IP TOS/TC are masked out — appendPayload
// merges CE into the seed; flushSlot rewrites lengths.
// fields are masked out (flushSlot rewrites them), but the IP-level ECN
// codepoint is compared (via ipHeadersMatch) so segments with differing ECN
// don't coalesce, matching kernel GRO.
func udpHeadersMatch(a, b []byte, isV6 bool, ipHdrLen int) bool {
if len(a) != len(b) {
return false
+23 -18
View File
@@ -261,32 +261,37 @@ func TestUDPCoalescerCapsAtMaxSegs(t *testing.T) {
}
}
// CE marks on appended segments must be merged into the seed's IP TOS.
func TestUDPCoalescerMergesCEMark(t *testing.T) {
// Differing IP ECN codepoints must not coalesce: udpHeadersMatch compares
// the full ToS byte (matching kernel GRO). A CE-marked datagram mid-run
// seals the Not-ECT chain and seeds a fresh superpacket that keeps CE; the
// trailing Not-ECT datagram seeds another.
func TestUDPCoalescerDifferingECNReseeds(t *testing.T) {
w := &fakeTunWriter{gsoEnabled: true}
c := NewUDPCoalescer(w, NewArena(0))
pay := make([]byte, 800)
pkt0 := buildUDPv4(1000, 53, pay) // ECN=00
pkt0 := buildUDPv4(1000, 53, pay) // ECN=00 (Not-ECT)
pkt1 := buildUDPv4(1000, 53, pay)
pkt1[1] = 0x03 // CE
pkt2 := buildUDPv4(1000, 53, pay)
if err := c.Commit(pkt0); err != nil {
t.Fatal(err)
}
if err := c.Commit(pkt1); err != nil {
t.Fatal(err)
}
if err := c.Commit(pkt2); err != nil {
t.Fatal(err)
pkt1[1] = 0x03 // CE
pkt2 := buildUDPv4(1000, 53, pay) // ECN=00 again
for _, p := range [][]byte{pkt0, pkt1, pkt2} {
if err := c.Commit(p); err != nil {
t.Fatal(err)
}
}
if err := c.Flush(); err != nil {
t.Fatal(err)
}
if len(w.gsoWrites) != 1 {
t.Fatalf("want 1 merged gso write, got %d (plain=%d)", len(w.gsoWrites), len(w.writes))
if len(w.gsoWrites) != 3 {
t.Fatalf("want 3 separate seeds (differing ECN), got %d (plain=%d)", len(w.gsoWrites), len(w.writes))
}
if w.gsoWrites[0].hdr[1]&0x03 != 0x03 {
t.Errorf("CE not merged into seed (tos=%#x)", w.gsoWrites[0].hdr[1])
wantECN := []byte{0x00, 0x03, 0x00}
for i, g := range w.gsoWrites {
if len(g.pays) != 1 {
t.Errorf("gso %d pay count=%d want 1", i, len(g.pays))
}
if got := g.hdr[1] & 0x03; got != wantECN[i] {
t.Errorf("gso %d ECN=%#x want %#x", i, got, wantECN[i])
}
}
}
@@ -326,7 +331,7 @@ func TestUDPCoalescerIPv6Coalesces(t *testing.T) {
}
}
// DSCP differences must reseed (headers don't match outside ECN).
// DSCP differences must reseed: udpHeadersMatch compares the full ToS byte.
func TestUDPCoalescerDSCPMismatchReseeds(t *testing.T) {
w := &fakeTunWriter{gsoEnabled: true}
c := NewUDPCoalescer(w, NewArena(0))