perf: port upstream gopacket removal + IpAssoc wiring (ee7a476)#263
Open
full-bars wants to merge 9 commits into
Open
perf: port upstream gopacket removal + IpAssoc wiring (ee7a476)#263full-bars wants to merge 9 commits into
full-bars wants to merge 9 commits into
Conversation
Adds 11 new files from upstream connect main (ahead of v2026.7.6-985989570): New files — no behavioural changes to existing code: - ip_assoc.go (1135L) — activity-based IP co-association matrix - ip_block_action.go (570L) — block action overrides for security policy - memory_budget.go (56L) — process-wide memory budget scaling - transfer_memory_budget.go (74L) — shared byte budget for sequences - transfer_contract_stats.go (164L) — per-contract usage event callbacks - egress.go / egress_net.go / egress_other.go / egress_windows.go (181L) — egress interface binding - transfer_peer_manager.go (166L) — peer tracking from control messages - race_on_test.go / race_off_test.go — test helpers for race detection Supporting additions to existing files: - protocol/frame.pb.go, protocol/transfer.pb.go — regenerated for new message types - transfer.go — Peer, NetworkPeer types - util.go — memoryShedders, AddMemoryShedder, ShedMemory - ip_remote_multi_client.go — ServerNameLookup interface, EventEpoch field + default - transfer_contract_manager.go — SignStoredContract, VerifyStoredContract, NetworkEventTimeChangeHmac field, ContractStatsEpoch field, contract stats struct fields New files compile against the fork's existing interfaces. Test files that depend on not-yet-ported methods (block actions, packet stats, IpAssoc wiring) are deferred to later PRs.
Changes ReceiveFunction signature from `provideMode protocol.ProvideMode` to `peer Peer` (upstream 83dc999). Updates all 7 source file signatures, 4 test file lambdas, and connectctl. Adds ProvideMode_Network to the contract-manager pause filter so network peers don't fall back to stream contracts when paused.
Replaces gopacket-based packet parsing/building with manual binary encoding on all hot paths in ip.go. Adds packet stats, ipPath caching, and lock-free multiClientConfig refactoring in ip_remote_multi_client.go. - Removes github.com/google/gopacket dependency from ip.go - Custom IPProtocol/UDPPort/TCPPort types + manual IPv4/IPv6/TCP/UDP parse and write functions - New fields: ipAssoc, blockAction state, packetStats on RemoteUserNatMultiClient - Atomic config pointer replaces separate performanceProfile + localSecurityBypass fields - Test file updated to use new types
runShard's bare `return` on parse failure exited the whole loop, firing defer self.cancel() and killing the shard permanently on a single malformed TCP/UDP segment from any client (remote DoS). Changed to continue so only the bad packet is dropped. Also add the len(ipPacket) == 0 guard in ParseIpPathWithPayload that upstream's reference port includes, preventing an index-out-of-range panic on an empty packet.
…gaps
Five issues found in independent review of the gopacket-removal port:
1. tcpPacket()/SynAck() computed the TCP checksum over the header only,
excluding payload/options. Every TCP data segment sent to clients
carried an invalid checksum and would be silently dropped by
standard client network stacks. Fixed to checksum the full segment,
matching the UDP path.
2. parseWindowScaleOpts didn't handle TCP option kinds 0 (EOL) and 1
(NOP), which have no length byte. Since real SYNs almost always
place a NOP before the Window Scale option, the parser desynced and
missed WS for nearly all real clients. Fixed to handle both.
3. SynAck() allocated an unpadded options buffer (3 bytes) while
writeTcpHeader's data-offset field rounds up to a 4-byte boundary
(24), producing a segment 1 byte shorter than its own header claims.
Receiving stacks reject this as truncated. Fixed to pad options to
a 4-byte boundary (zero-fill reads as EOL).
4. tcpSend's RST branch didn't return ipPacket to the message pool on
an existing-sequence teardown, unlike the sibling no-syn-drop branch.
Fixed to return it.
5. runShard's ipVersion switch had no default case, leaking the pool
buffer for any packet with a first nibble outside {4, 6}. Fixed to
add one, matching the existing per-protocol default branches.
- Extract the TCP window-scale option parser from TcpSequence.Run()'s
inline closure to a package-level exported function for testability.
- Add ip_packet_test.go with 33 tests covering:
- checksumAdd/checksumFinish (RFC 1071)
- transportChecksum (pseudo-header + segment)
- TCP checksum scope (payload, options)
- ConnectionState PureAck/RstAck packet construction
- parseIpv4/parseIpv6/parseUdpPacket/parseTcpPacket round trips
- ParseTcpWindowScaleOpts (nil, empty, WS, cap-at-14, NOP, EOL,
unknown options, multi-NOP)
- writeIpv4Header/writeIpv6Header/writeTcpHeader round trips
- flowHash (IPv4/IPv6 consistency, short packets)
- malformed packet guards (nil, empty, invalid data offset)
- SynAck data-offset/segment-length consistency
- UDP checksum non-zero invariant
- Fix TestChecksumAddAndFinish expected value.
Adds unit-level checksumAdd/checksumFinish coverage, PureAck/RstAck construction tests, parseIpv4/parseIpv6/parseUdpPacket/parseTcpPacket round trips, writeIpv4Header/writeIpv6Header/writeTcpHeader round trips, malformed/too-short input guards, empty-packet guard, and flow-hash edge cases, on top of the checksum-scope and window-scale option tests from the prior commit. Fixed one flaky test in the process: TestWriteIpv4RoundTrip verified the IPv4 checksum over the full allocated buffer (including 8 bytes of unrelated UDP-header space past the 20-byte IP header), which only holds when the message pool happens to hand back a zeroed buffer. Pool buffers aren't guaranteed zeroed on reuse, so this failed intermittently depending on prior test pool state. Scoped the verification to the actual 20-byte IPv4 header, matching what writeIpv4Header itself checksums. Verified with -shuffle=on across multiple runs after the fix.
full-bars
force-pushed
the
perf/upstream-gopacket-removal
branch
2 times, most recently
from
f6728b3 to
c4065b4
Compare
Covers c4065b4's fix directly: StreamState.DataPackets (UDP) and ConnectionState.DataPackets (TCP) both now error on mtu <= header size instead of divide-by-zero / negative-capacity panicking. Verified against the unfixed code — reproduces the exact "makeslice: cap out of range" panic from the audit report.
full-bars
force-pushed
the
perf/upstream-gopacket-removal
branch
from
4519357 to
069a964
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the gopacket elimination, packet stats infrastructure, and
IpAssoc/block-action wiring from upstream commit
ee7a476(perf fixesit touches the fork's most-customized files (ip.go, ip_remote_multi_client.go).
Depends on PR #261's new files (ip_assoc.go, ip_block_action.go,
memory_budget.go, transfer_contract_stats.go).
Changes by file
ip.go (~1042 lines changed)
gopacket removal (all hot paths):
github.com/google/gopacketandgopacket/layersimportsIPProtocol,UDPPort,TCPPorttypes replace gopacket typesparseIpv4,parseIpv6,parseUdpPacket,parseTcpPacketwriteIpv4Header,writeIpv6Header,writeTcpHeaderchecksumAdd,checksumFinish,transportChecksumrunShard,ParseIpPathWithPayload) uses manual parsingDataPackets,SynAck,PureAck, etc.) use manual binary encodingip_remote_multi_client.go (+63 lines)
multiClientConfigatomic pointer replaces separateperformanceProfile+localSecurityBypassfieldsSetServerNameLookup()— new method for SNI-free ServerName affinity routingconfig.Load()ip_udp_write_pipeline_leak_test.go
UDPPort/parsedUdptypes instead of gopacketFork features preserved
SecurityPolicyinterface (unaryInspect, no ctx, no Refresh methods)IngressSecurityPolicyGenerator/EgressSecurityPolicyGeneratorfieldsBuild verification
go build .— cleango build ./provider/— cleango vet .— cleango test -short ./provider/— 7.5s, all pass